Fix #6227: guard POJOPropertiesCollector.collectAll() against concurrent calls

[pjfanning]

Fixes #6227

Three races on a shared BeanDescription, each with a test that fails without the fix.

1. findProperties() → POJOPropertiesCollector.collectAll()

collectAll() is not just building the local props map: it also mutates collector instance state (_potentialCreators = new PotentialCreators(), _fieldRenameMappings, the any-getter/any-setter LinkedLists, _creatorProperties, ...). Two threads that both observe _collected == false run it concurrently on the same instance:

• IllegalArgumentException: Conflicting property-based creators — both threads register the same creator constructor into the same PotentialCreators
• ConcurrentModificationException — _sortProperties() iterates a list the other thread is still appending to

Fix: _collected becomes volatile and collectAll() synchronized, with an early return if another thread finished collection while we waited on the lock. The existing if (!_collected) collectAll(); accessors thus become a standard double-checked pattern. BasicBeanDescription._properties() is synchronized too, closing the (benign) race where two threads each fetched their own list copy. This only guards the lazy initialization: the returned List itself is not thread-safe (concurrent addProperty() / removeProperty() vs. readers is out of scope here).

2. findDefaultViews() returns null views

Separate bug in the same class, found while stress-testing the above:

if (!_defaultViewsResolved) {
    _defaultViewsResolved = true;                 // flag set FIRST
    Class<?>[] def = ... findViews(_classInfo);   // ...then the work
    _defaultViews = def;                          // ...then the value
}
return _defaultViews;                             // second thread returns null

A second thread sees the flag set and returns null. This is a plain ordering bug rather than a memory-visibility one, so it does not depend on the memory model — and it fails silently: @JsonView filtering is then applied as if the class had no view annotation, instead of throwing.

Fix: assign the value before the flag, and synchronize the method to match _properties().

3. findJsonValueAccessor() / findJsonKeyAccessor() → _resolveFieldVsGetter()

When both a field and its getter carry @JsonValue (or @JsonKey), POJOPropertiesCollector.getJsonValueAccessor() / getJsonKeyAccessor() resolve the conflict lazily, on each call, by removing entries from the shared accessor list (accessors.remove(0) / remove(1)). This runs after collection and outside the collectAll() lock, so two threads that both see size() > 1 remove entries concurrently:

• IndexOutOfBoundsException: Index: 1, Size: 1 — one thread removed an entry between the other's size check and get(1)
• IndexOutOfBoundsException: Index: 0, Size: 0 — both threads removed an entry, leaving the list empty

Fix: both getters are synchronized (same monitor as collectAll(), which is reentrant). Resolution stays lazy, so the "Multiple 'as-value' properties" error is still reported on access rather than during collection.

Scope

Jackson does not share BeanDescriptions across threads itself (BasicClassIntrospector constructs them per call in 2.x; in 3.x ClassIntrospector.forOperation() gives each context its own per-operation cache), so both bugs need user code or a framework to hold one and use it from several threads — exactly the issue's repro. Nothing documents that as unsupported, and both fixes are cheap. synchronized is not part of the method descriptor, so there is no API/binary change.

Tests

• BeanDescriptionConcurrent6227Test — ports the issue's repro (16 threads, 50 rounds, CyclicBarrier), using a @JsonCreator POJO rather than a record so it runs on Java 8. Fails on the first round without the fix.
• BeanDescriptionConcurrent6227Test.testConcurrentFindJsonValueAccessor() / testConcurrentFindJsonKeyAccessor() — same harness, with the annotation on both a field and its getter; asserts the getter wins. Fails on every run without the fix.
• BeanDescriptionDefaultViewsRaceTest — deterministic, not probabilistic: an AnnotationIntrospector blocks inside the first findViews() call, holding the window open, so the second thread observes null on every run without the fix.

Full PrimarySuite green with all three changes (3903 tests).

Beyond these, I stress-tested ordinary shared-ObjectMapper use under load (~250k ops): cyclic/self-referencing graphs, polymorphic (Id.CLASS, EXTERNAL_PROPERTY, defaultImpl), generics/TypeReference, root-name wrapping, dates, object ids, views, filters, any-getter/setter, builders, tree model, convertValue, MappingIterator, shared ObjectReader/ObjectWriter, and LRU eviction pressure — including comparing 72,000 concurrent outputs against single-threaded golden strings to catch silent corruption. No further failures surfaced, so nothing else is changed here.

Branches

Targeted at 2.21 for merge-up. The issue was reported against 3.1.5 and all changes apply to 3.1 as well (the first was originally developed and verified there). The javadoc notes deliberately cite only [databind#6227] and not a version, so the forward merge should be conflict-free apart from release-notes/VERSION.

🤖 Generated with Claude Code

[github-actions]

🧪 Code Coverage Report

Metric	Coverage	Change
Instructions	81.48%	📈 +0.040%
Branches	74.82%	📈 +0.060%

Coverage data generated from JaCoCo test results

[pjfanning]

A note on the choice of synchronized here, since it sometimes raises eyebrows:

• The lock is uncontended in practice (taken once per collector, and only contended at all in the "shared BeanDescription" case this issue is about), so it is essentially free on any modern JVM.
• The virtual-thread pinning concern with synchronized (JDK 21–23) was removed by JEP 491 in JDK 24. Even on 21–23, pinning only matters if the thread blocks while holding the monitor; collectAll() does in-memory reflection only, no I/O, so the window is tiny.
• It matches existing practice in databind (e.g. the deserializer/serializer caches guard one-time lazy construction the same way).

Alternatives if a reviewer prefers: a private Object lock field instead of this (avoids any risk of external code locking on the collector), or a ReentrantLock. Happy to switch if there's a preference, but I don't think either buys anything here.

[gitar-bot]

Code Review ✅ Approved

🟡 Medium risk · Synchronization and lazy-state changes alter concurrent property and view introspection behavior

Adds synchronized guards and volatile flag to POJOPropertiesCollector.collectAll() and BasicBeanDescription.findDefaultViews() to prevent concurrent mutation races on shared BeanDescription instances. Fixes IllegalArgumentException from duplicate creator registration and ConcurrentModificationException from list iteration during concurrent collection, with deterministic test coverage for both scenarios. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[cowtowncoder]

Fixed in 2.21 for 2.21.8; merged forward to 2.22, 2.x, 3.1, 3.2, 3.x.