Skip to content

Commit 6246748

Browse files
cowtowncoderclaude
andcommitted
Merge branch '2.x' into 3.1
# Conflicts: # src/main/java/tools/jackson/databind/introspect/BasicBeanDescription.java # src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java # src/test/java/tools/jackson/databind/introspect/BeanDescriptionConcurrent6227Test.java # src/test/java/tools/jackson/databind/introspect/BeanDescriptionDefaultViewsRaceTest.java Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 parents b45bc44 + b4bd3ef commit 6246748

8 files changed

Lines changed: 266 additions & 7 deletions

File tree

‎release-notes/CREDITS‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,3 +442,8 @@ Francesco Nigro (@franz1981)
442442
* Reported #6204: Avoid quadratic forward-reference resolution in Collection/Map
443443
deserializers
444444
[3.1.7]
445+
446+
Sergey Lappo (@sergeylappo)
447+
* Reported #6227: `BeanDescription` is not thread-safe when called concurrently on the
448+
same instance (`findProperties()`, `findDefaultViews()`)
449+
[3.1.8]

‎release-notes/CREDITS-2.x‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,3 +2112,8 @@ Christian Danner cdadac
21122112
* Reported #3064: `@JsonPropertyOrder(alphabetic=true)` is ignored in case indices
21132113
are defined for `@JsonProperty` -- add `MapperFeature.SORT_PROPERTIES_BY_INDEX`
21142114
[2.22.0]
2115+
2116+
Sergey Lappo (@sergeylappo)
2117+
* Reported #6227: `BeanDescription` is not thread-safe when called concurrently on the
2118+
same instance (`findProperties()`, `findDefaultViews()`)
2119+
[2.21.8]

‎release-notes/VERSION‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ No changes since 3.1
1414
#6206: Report cycle through `@JsonValue` accessor as `DatabindException`
1515
(instead of `StackOverflowError`)
1616
(fix by @pjfanning, w/ Claude code)
17+
#6227: `BeanDescription` is not thread-safe when called concurrently on the
18+
same instance (`findProperties()`, `findDefaultViews()`)
19+
(reported by @sergeylappo)
20+
(fix by @pjfanning, w/ Claude code)
1721
#6232: Validate `StreamReadConstraints` number length when coercing `StringNode`
1822
to number
1923
(fix by Aysha A-Z)

‎release-notes/VERSION-2.x‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ No changes since 2.22
1313
#6206: Report cycle through `@JsonValue` accessor as `JsonMappingException`
1414
(instead of `StackOverflowError`)
1515
(fix by @pjfanning, w/ Claude code)
16+
#6227: `BeanDescription` is not thread-safe when called concurrently on the
17+
same instance (`findProperties()`, `findDefaultViews()`)
18+
(reported by @sergeylappo)
19+
(fix by @pjfanning, w/ Claude code)
1620

1721
2.22.3 (21-Sep-2026)
1822

@@ -81,6 +85,10 @@ No changes since 2.22
8185
#6206: Report cycle through `@JsonValue` accessor as `JsonMappingException`
8286
(instead of `StackOverflowError`)
8387
(fix by @pjfanning, w/ Claude code)
88+
#6227: `BeanDescription` is not thread-safe when called concurrently on the
89+
same instance (`findProperties()`, `findDefaultViews()`)
90+
(reported by @sergeylappo)
91+
(fix by @pjfanning, w/ Claude code)
8492

8593
2.21.7 (21-Sep-2026)
8694

‎src/main/java/tools/jackson/databind/introspect/BasicBeanDescription.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,9 @@ public static BasicBeanDescription forOtherUse(MapperConfig<?> config,
147147
return new BasicBeanDescription(config, type, ac);
148148
}
149149

150-
protected List<BeanPropertyDefinition> _properties() {
150+
// [databind#6227]: synchronized to guard lazy initialization in case instance
151+
// is shared across threads; returned List itself is NOT thread-safe
152+
protected synchronized List<BeanPropertyDefinition> _properties() {
151153
if (_properties == null) {
152154
_properties = _propCollector.getProperties();
153155
}
@@ -376,19 +378,22 @@ public AnnotatedMethod findMethod(String name, Class<?>[] paramTypes) {
376378
/**********************************************************************
377379
*/
378380

381+
// [databind#6227]: synchronized in case instance is shared across threads
379382
@Override
380-
public Class<?>[] findDefaultViews()
383+
public synchronized Class<?>[] findDefaultViews()
381384
{
382385
if (!_defaultViewsResolved) {
383-
_defaultViewsResolved = true;
384386
Class<?>[] def = _intr.findViews(_config, _classInfo);
385387
// one more twist: if default inclusion disabled, need to force empty set of views
386388
if (def == null) {
387389
if (!_config.isEnabled(MapperFeature.DEFAULT_VIEW_INCLUSION)) {
388390
def = NO_VIEWS;
389391
}
390392
}
393+
// 22-Sep-2026: [databind#6227] MUST assign value before flag: otherwise another
394+
// thread may see "resolved" flag set and return not-yet-assigned `null`
391395
_defaultViews = def;
396+
_defaultViewsResolved = true;
392397
}
393398
return _defaultViews;
394399
}

‎src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,12 @@ public class POJOPropertiesCollector
7171
/**
7272
* State flag we keep to indicate whether actual property information
7373
* has been collected or not.
74+
*<p>
75+
* NOTE: {@code volatile} so that the "collected" state (and,
76+
* transitively, results assigned before it) is visible across threads in case
77+
* a {@link BeanDescription} instance is shared, see [databind#6227].
7478
*/
75-
protected boolean _collected;
79+
protected volatile boolean _collected;
7680

7781
/**
7882
* Set of logical property information collected so far.
@@ -212,7 +216,11 @@ public Map<Object, AnnotatedMember> getInjectables() {
212216
return _injectables;
213217
}
214218

215-
public AnnotatedMember getJsonKeyAccessor() {
219+
/**
220+
* NOTE: {@code synchronized} since resolution of conflicting accessors
221+
* modifies the accessor list (see [databind#6227]).
222+
*/
223+
public synchronized AnnotatedMember getJsonKeyAccessor() {
216224
if (!_collected) {
217225
collectAll();
218226
}
@@ -231,7 +239,11 @@ public AnnotatedMember getJsonKeyAccessor() {
231239
return null;
232240
}
233241

234-
public AnnotatedMember getJsonValueAccessor()
242+
/**
243+
* NOTE: {@code synchronized} since resolution of conflicting accessors
244+
* modifies the accessor list (see [databind#6227]).
245+
*/
246+
public synchronized AnnotatedMember getJsonValueAccessor()
235247
{
236248
if (!_collected) {
237249
collectAll();
@@ -373,10 +385,19 @@ public JsonFormat.Value getFormatOverrides() {
373385

374386
/**
375387
* Internal method that will collect actual property information.
388+
*<p>
389+
* NOTE: {@code synchronized} since although instances are
390+
* not designed to be shared across threads, if they are, concurrent collection
391+
* would corrupt internal state (see [databind#6227]).
376392
*/
377-
protected void collectAll()
393+
protected synchronized void collectAll()
378394
{
379395
//System.out.println(" PojoPropsCollector.collectAll() for "+_classDef.getRawType().getName());
396+
// [databind#6227]: another thread may have completed collection while we
397+
// were waiting for the lock
398+
if (_collected) {
399+
return;
400+
}
380401
_potentialCreators = new PotentialCreators();
381402

382403
// First: gather basic accessors
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package tools.jackson.databind.introspect;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import java.util.Queue;
6+
import java.util.concurrent.*;
7+
8+
import org.junit.jupiter.api.Test;
9+
10+
import com.fasterxml.jackson.annotation.JsonCreator;
11+
import com.fasterxml.jackson.annotation.JsonKey;
12+
import com.fasterxml.jackson.annotation.JsonProperty;
13+
import com.fasterxml.jackson.annotation.JsonValue;
14+
15+
import tools.jackson.databind.BeanDescription;
16+
import tools.jackson.databind.ObjectMapper;
17+
import tools.jackson.databind.ObjectMapperTestAccess;
18+
import tools.jackson.databind.testutil.DatabindTestUtil;
19+
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
22+
// [databind#6227]
23+
public class BeanDescriptionConcurrent6227Test extends DatabindTestUtil
24+
{
25+
static class Probe {
26+
public final String id;
27+
public final int count;
28+
29+
@JsonCreator
30+
public Probe(@JsonProperty("id") String id, @JsonProperty("count") int count) {
31+
this.id = id;
32+
this.count = count;
33+
}
34+
}
35+
36+
// Both field and getter annotated: resolution modifies accessor list
37+
static class ValueProbe {
38+
@JsonValue
39+
public String value = "x";
40+
41+
@JsonValue
42+
public String getValue() { return value; }
43+
}
44+
45+
static class KeyProbe {
46+
@JsonKey
47+
public String key = "x";
48+
49+
@JsonKey
50+
public String getKey() { return key; }
51+
}
52+
53+
@FunctionalInterface
54+
interface DescAction {
55+
void run(BeanDescription desc) throws Exception;
56+
}
57+
58+
@Test
59+
public void testConcurrentFindProperties() throws Exception
60+
{
61+
ObjectMapper mapper = newJsonMapper();
62+
63+
// Repeat a number of times to increase odds of hitting race
64+
for (int round = 0; round < 50; ++round) {
65+
final BeanDescription shared = ObjectMapperTestAccess.beanDescriptionForSer(mapper, Probe.class);
66+
_runRound(shared, 16, BeanDescription::findProperties);
67+
}
68+
}
69+
70+
@Test
71+
public void testConcurrentFindJsonValueAccessor() throws Exception
72+
{
73+
ObjectMapper mapper = newJsonMapper();
74+
75+
for (int round = 0; round < 50; ++round) {
76+
final BeanDescription shared = ObjectMapperTestAccess.beanDescriptionForSer(mapper, ValueProbe.class);
77+
// Getter has precedence over field
78+
_runRound(shared, 16, desc -> assertTrue(
79+
desc.findJsonValueAccessor() instanceof AnnotatedMethod));
80+
}
81+
}
82+
83+
@Test
84+
public void testConcurrentFindJsonKeyAccessor() throws Exception
85+
{
86+
ObjectMapper mapper = newJsonMapper();
87+
88+
for (int round = 0; round < 50; ++round) {
89+
final BeanDescription shared = ObjectMapperTestAccess.beanDescriptionForSer(mapper, KeyProbe.class);
90+
// Getter has precedence over field
91+
_runRound(shared, 16, desc -> assertTrue(
92+
desc.findJsonKeyAccessor() instanceof AnnotatedMethod));
93+
}
94+
}
95+
96+
private void _runRound(final BeanDescription shared, int parallelism,
97+
final DescAction action) throws Exception
98+
{
99+
final CyclicBarrier barrier = new CyclicBarrier(parallelism);
100+
final Queue<Throwable> errors = new ConcurrentLinkedQueue<>();
101+
ExecutorService pool = Executors.newFixedThreadPool(parallelism);
102+
List<Future<?>> futures = new ArrayList<>();
103+
for (int i = 0; i < parallelism; i++) {
104+
futures.add(pool.submit(() -> {
105+
try {
106+
barrier.await();
107+
action.run(shared);
108+
} catch (Throwable e) {
109+
errors.add(e);
110+
}
111+
}));
112+
}
113+
for (Future<?> f : futures) {
114+
f.get(30, TimeUnit.SECONDS);
115+
}
116+
pool.shutdown();
117+
if (!errors.isEmpty()) {
118+
throw new AssertionError("Failed with "+errors.size()+" errors, first: "+errors.peek(), errors.peek());
119+
}
120+
assertTrue(errors.isEmpty());
121+
}
122+
}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package tools.jackson.databind.introspect;
2+
3+
import java.util.concurrent.*;
4+
import java.util.concurrent.atomic.AtomicReference;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
import com.fasterxml.jackson.annotation.JsonView;
9+
10+
import tools.jackson.databind.BeanDescription;
11+
import tools.jackson.databind.ObjectMapper;
12+
import tools.jackson.databind.ObjectMapperTestAccess;
13+
import tools.jackson.databind.cfg.MapperConfig;
14+
import tools.jackson.databind.testutil.DatabindTestUtil;
15+
16+
import static org.junit.jupiter.api.Assertions.*;
17+
18+
// [databind#6227] `findDefaultViews()` set its "resolved" flag BEFORE computing the
19+
// value, so a second thread could observe the flag and return `null` views.
20+
public class BeanDescriptionDefaultViewsRaceTest extends DatabindTestUtil
21+
{
22+
static class Views { static class Pub { } }
23+
24+
@JsonView(Views.Pub.class)
25+
static class ViewedBean {
26+
public String a = "a";
27+
public String b = "b";
28+
}
29+
30+
/**
31+
* Introspector that blocks inside the very first {@code findViews()} call, holding
32+
* open the window between the flag write and the value write.
33+
*/
34+
static class BlockingIntrospector extends JacksonAnnotationIntrospector
35+
{
36+
private static final long serialVersionUID = 1L;
37+
38+
final CountDownLatch inside = new CountDownLatch(1);
39+
final CountDownLatch release = new CountDownLatch(1);
40+
final Semaphore firstCall = new Semaphore(1);
41+
42+
@Override
43+
public Class<?>[] findViews(MapperConfig<?> config, Annotated a) {
44+
if (firstCall.tryAcquire()) { // only the first caller blocks
45+
inside.countDown();
46+
try {
47+
release.await(10, TimeUnit.SECONDS);
48+
} catch (InterruptedException e) {
49+
Thread.currentThread().interrupt();
50+
}
51+
}
52+
return super.findViews(config, a);
53+
}
54+
}
55+
56+
@Test
57+
public void testConcurrentFindDefaultViews() throws Exception
58+
{
59+
final BlockingIntrospector intr = new BlockingIntrospector();
60+
ObjectMapper mapper = jsonMapperBuilder()
61+
.annotationIntrospector(intr)
62+
.build();
63+
final BeanDescription desc = ObjectMapperTestAccess.beanDescriptionForSer(mapper, ViewedBean.class);
64+
65+
final AtomicReference<Class<?>[]> fromA = new AtomicReference<>();
66+
final AtomicReference<Class<?>[]> fromB = new AtomicReference<>();
67+
68+
ExecutorService pool = Executors.newFixedThreadPool(2);
69+
try {
70+
Future<?> a = pool.submit(() -> fromA.set(desc.findDefaultViews()));
71+
// wait until thread A is INSIDE findViews(), i.e. mid-resolution
72+
assertTrue(intr.inside.await(10, TimeUnit.SECONDS), "thread A never reached findViews()");
73+
Future<?> b = pool.submit(() -> fromB.set(desc.findDefaultViews()));
74+
// give B a chance to read the half-resolved state, then let A finish
75+
Thread.sleep(100L);
76+
intr.release.countDown();
77+
a.get(10, TimeUnit.SECONDS);
78+
b.get(10, TimeUnit.SECONDS);
79+
} finally {
80+
pool.shutdownNow();
81+
}
82+
83+
assertNotNull(fromA.get(), "views resolved by first thread should not be null");
84+
assertNotNull(fromB.get(),
85+
"second thread saw the 'resolved' flag before the value was assigned, and got null views");
86+
assertArrayEquals(fromA.get(), fromB.get());
87+
assertEquals(Views.Pub.class, fromB.get()[0]);
88+
}
89+
}

0 commit comments

Comments
 (0)