#3359 fix: ConcurrentModificationException regression in schema getTypes()

[robfrank]

Summary

• Commit 9959c82 ("remove duplicates from types returned") introduced a regression where LocalSchema.getTypes() wraps HashMap.values() in a LinkedHashSet, iterating over a live view that throws ConcurrentModificationException when another thread modifies the schema concurrently
• Fix: change the types map from HashMap to ConcurrentHashMap, whose iterators are weakly consistent and safe for concurrent reads
• The deduplication logic from the previous fix is preserved

Test plan

• Issue3359Test passes (concurrent read queries with schema modification)
• SchemaTest passes (7 tests)

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a concurrency regression by making the schema's type storage thread-safe. It prevents ConcurrentModificationException when LocalSchema.getTypes() is accessed concurrently with schema modifications, ensuring system stability under multi-threaded operations.

Highlights

• ConcurrentModificationException Fix: Resolved a ConcurrentModificationException that occurred in LocalSchema.getTypes() by replacing the HashMap used for storing types with a ConcurrentHashMap, ensuring thread-safe access during concurrent schema modifications.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• engine/src/main/java/com/arcadedb/schema/LocalSchema.java
  • Changed the types map from HashMap to ConcurrentHashMap to ensure thread-safe access during concurrent operations.

Activity

• Issue3359Test passed, confirming the fix for concurrent read queries with schema modification.
• SchemaTest passed with 7 tests, ensuring no regressions were introduced.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for d1b4d56.

🟢 All jobs passed!

But CI Insights is watching 👀

[claude]

Code Review

Summary

This is a clean, minimal fix for a real concurrency regression. The approach is correct and consistent with existing patterns in the class.

---

Positive Aspects

Consistency with existing patterns — functionLibraries (line 116) and migratedFileIds (line 117) already use ConcurrentHashMap, so this change is idiomatic within LocalSchema.

Minimal change — Only the necessary line is touched, and deduplication via LinkedHashSet in getTypes() (needed because aliases map to the same DocumentType instance) is preserved.

Good regression test — Issue3359Test reproduces the race correctly using CountDownLatch to synchronize thread startup, an AtomicReference to capture exceptions, and a writer/reader pattern that mirrors the original report.

---

Minor Observations

Weakly-consistent iteration semantics (acceptable, worth a comment)

ConcurrentHashMap iterators are weakly consistent: they reflect the map state at iteration time and may or may not include mutations applied concurrently. For getTypes() this is the right trade-off — callers should already expect a transient snapshot — but a brief Javadoc note would help future readers understand why this is intentional.

readConfiguration() still has a brief empty-map window

The method is synchronized (line 1330), but it calls types.clear() and then repopulates. Concurrent readers calling getTypes() outside any lock will see an empty collection during that window. This is a separate, pre-existing issue and out of scope here — worth tracking separately.

Test placement

Issue3359Test lives in com.arcadedb.query.opencypher even though it tests schema concurrency. The Cypher query is merely the trigger; com.arcadedb.schema or a general engine package could also be appropriate. No hard blocker, but the current location may surprise someone looking for schema concurrency tests later.

Other maps in the same class

bucketMap, indexMap, triggers, materializedViews, and continuousAggregates are still plain HashMap/LinkedHashMap (lines 95-102). They appear to be protected by synchronized methods on most write paths, so they likely rely on a different concurrency strategy. This is out of scope for this PR, but worth a follow-up audit to confirm consistency.

---

Bottom Line

The fix is correct, targeted, and consistent with existing code. The regression test is well-constructed. The observations above are all minor; none of them block merging.

[gemini-code-assist]

Code Review

This pull request addresses a ConcurrentModificationException regression in LocalSchema.getTypes() by changing the types map from a HashMap to a ConcurrentHashMap. This change is correct, as ConcurrentHashMap is designed for concurrent access and its iterators do not throw ConcurrentModificationException. The fix is minimal and effectively resolves the described issue without affecting other logic.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -10.55%	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (ac3f5c0)	103683	77518	74.76%
Head commit (d1b4d56)	134242 (+30559)	86205 (+8687)	64.22% (-10.55%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3590)	1	1	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.01%. Comparing base (ac3f5c0) to head (d1b4d56).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3590      +/-   ##
==========================================
- Coverage   65.81%   65.01%   -0.81%     
==========================================
  Files        1514     1514              
  Lines      103683   103683              
  Branches    21457    21457              
==========================================
- Hits        68239    67408     -831     
- Misses      26193    27098     +905     
+ Partials     9251     9177      -74     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.