synchronize access to IdentityHashMap instances#1787
Conversation
PR Summary
These changes collectively improve concurrent usage of these classes and reduce the chance of unexpected behavior in multithreaded environments. This PR ensures that simultaneous modifications from different threads are properly handled, increasing the reliability of the software. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1787 +/- ##
============================================
- Coverage 92.39% 92.36% -0.03%
+ Complexity 3448 3447 -1
============================================
Files 339 339
Lines 6794 6794
Branches 670 670
============================================
- Hits 6277 6275 -2
- Misses 352 353 +1
- Partials 165 166 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses concurrency-related corruption of several identity-keyed caches by synchronizing access to IdentityHashMap instances across the codebase (aligning with the reported ClassCastException seen in concurrent runs).
Changes:
- Wrap multiple
IdentityHashMapcaches withCollections.synchronizedMap(...)to make basic map operations thread-safe. - Apply the same synchronization approach consistently across provider caching, locale chain caching, reflection/method caches, and transformer constructor/schema caches.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/net/datafaker/transformations/JavaObjectTransformer.java | Synchronizes static caches for schema-to-consumer and class-to-constructor lookups. |
| src/main/java/net/datafaker/service/FakerContext.java | Synchronizes static locale chain and locale normalization caches. |
| src/main/java/net/datafaker/service/FakeValuesService.java | Synchronizes the primitive-to-wrapper identity map. |
| src/main/java/net/datafaker/providers/base/ObjectMethods.java | Synchronizes method lookup caches keyed by class identity. |
| src/main/java/net/datafaker/providers/base/BaseFaker.java | Synchronizes the provider instance cache to avoid concurrent IdentityHashMap corruption. |
adef835 to
c357c55
Compare
... to avoid concurrency issues like in build https://github.com/datafaker-net/datafaker/actions/runs/23618303854/job/68791305528?pr=1785: ``` java.lang.ClassCastException: class net.datafaker.providers.base.Color cannot be cast to class net.datafaker.providers.base.Vehicle (net.datafaker.providers.base.Color and net.datafaker.providers.base.Vehicle are in unnamed module of loader 'app') at net.datafaker.providers.base.BaseProviders.vehicle(BaseProviders.java:506) at net.datafaker.providers.base.VehicleLocaleTest.lambda$localeProviderListTest$3(VehicleLocaleTest.java:23) ``` Class `IdentityHashMap` is not thread-safe, all accesses to this class should be synchronized.
c357c55 to
0834f1b
Compare
... to avoid concurrency issues like in build https://github.com/datafaker-net/datafaker/actions/runs/23618303854/job/68791305528?pr=1785:
Class
IdentityHashMapis not thread-safe, all accesses to this class should be synchronized.How to reproduce the problem
This code shows how to stably reproduce the issue with
IdentityHashMap: