Skip to content

Commit d664ab2

Browse files
bm1549claude
andcommitted
Add cross-thread stress tests and fix benchmark setup race
Add three targeted concurrency tests that exercise the exact cross-thread tag write pattern the JMH crossThread benchmark was measuring: - crossThreadSustainedNoCrash: 8 threads × 10k setTag on same span - ownerToSharedTransition: owner writes first, then 8 threads join - manySpansCrossThread: 10k short-lived spans tagged from 8 threads All pass, proving the production code handles cross-thread writes without NPE or structural corruption. Fix the crossThread benchmark: change SharedSpan @setup from Level.Invocation to Level.Iteration. With Level.Invocation, 8 threads raced to call setup() concurrently, causing NPE when state.span was transiently null between invocations — a benchmark harness bug, not a production code bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7b8dee3 commit d664ab2

File tree

2 files changed

+148
-1
lines changed

2 files changed

+148
-1
lines changed

dd-trace-core/src/jmh/java/datadog/trace/core/SpanTagBenchmark.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void setup() {
3939
public static class SharedSpan {
4040
AgentSpan span;
4141

42-
@Setup(Level.Invocation)
42+
@Setup(Level.Iteration)
4343
public void setup() {
4444
span = TRACER.startSpan("benchmark", "tag-benchmark-shared");
4545
}

dd-trace-core/src/test/java/datadog/trace/core/DDSpanContextConcurrencyTest.java

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,151 @@ void mixedMetricsAndTags() {
159159

160160
span.finish();
161161
}
162+
163+
/**
164+
* Reproduces the exact benchmark pattern: one span created by thread A, 8 threads calling setTag
165+
* concurrently. This is the pattern that the JMH crossThread benchmark uses, except the benchmark
166+
* failed because of a race in the JMH harness (SharedSpan.setup with Level.Invocation + 8
167+
* threads), not in the production code.
168+
*
169+
* <p>This test proves the production code handles this pattern without NPE or structural
170+
* corruption.
171+
*/
172+
@Test
173+
@DisplayName(
174+
"Cross-thread sustained: 8 threads setTag on same span for 10k iterations — no crash")
175+
void crossThreadSustainedNoCrash() throws Exception {
176+
int numThreads = 8;
177+
int iterationsPerThread = 10_000;
178+
ExecutorService executor = Executors.newFixedThreadPool(numThreads);
179+
180+
// Create span on main thread, then hand it to 8 other threads
181+
AgentSpan span = TRACER.startSpan("test", "crossSustained");
182+
CyclicBarrier barrier = new CyclicBarrier(numThreads);
183+
CopyOnWriteArrayList<Throwable> errors = new CopyOnWriteArrayList<>();
184+
185+
List<Future<?>> futures = new ArrayList<>();
186+
for (int t = 0; t < numThreads; t++) {
187+
futures.add(
188+
executor.submit(
189+
() -> {
190+
try {
191+
barrier.await(5, TimeUnit.SECONDS);
192+
for (int i = 0; i < iterationsPerThread; i++) {
193+
span.setTag("key", "value");
194+
}
195+
} catch (Throwable e) {
196+
errors.add(e);
197+
}
198+
}));
199+
}
200+
201+
for (Future<?> f : futures) {
202+
f.get(30, TimeUnit.SECONDS);
203+
}
204+
span.finish();
205+
executor.shutdown();
206+
207+
assertEquals(0, errors.size(), "Unexpected errors during cross-thread setTag: " + errors);
208+
assertEquals("value", span.getTag("key"));
209+
}
210+
211+
/**
212+
* Tests the transition from owner-thread to shared mode under concurrent writes: the creating
213+
* thread sets tags first, then 8 other threads join in. This exercises the STATE_OWNER →
214+
* STATE_SHARED transition while writes are in flight.
215+
*/
216+
@Test
217+
@DisplayName("Owner-to-shared transition under concurrent writes — no crash, all tags present")
218+
void ownerToSharedTransition() throws Exception {
219+
int numThreads = 8;
220+
int tagsPerThread = 1_000;
221+
ExecutorService executor = Executors.newFixedThreadPool(numThreads);
222+
223+
AgentSpan span = TRACER.startSpan("test", "ownerToShared");
224+
225+
// Owner thread writes first batch — these are on the fast path (no lock)
226+
for (int i = 0; i < 100; i++) {
227+
span.setTag("owner_" + i, "val_" + i);
228+
}
229+
230+
// Now launch 8 threads that also write — these trigger the transition to STATE_SHARED
231+
CyclicBarrier barrier = new CyclicBarrier(numThreads);
232+
CopyOnWriteArrayList<Throwable> errors = new CopyOnWriteArrayList<>();
233+
234+
List<Future<?>> futures = new ArrayList<>();
235+
for (int t = 0; t < numThreads; t++) {
236+
final int threadIdx = t;
237+
futures.add(
238+
executor.submit(
239+
() -> {
240+
try {
241+
barrier.await(5, TimeUnit.SECONDS);
242+
for (int i = 0; i < tagsPerThread; i++) {
243+
span.setTag("thread" + threadIdx + "_" + i, "v" + i);
244+
}
245+
} catch (Throwable e) {
246+
errors.add(e);
247+
}
248+
}));
249+
}
250+
251+
for (Future<?> f : futures) {
252+
f.get(30, TimeUnit.SECONDS);
253+
}
254+
span.finish();
255+
executor.shutdown();
256+
257+
assertEquals(0, errors.size(), "Unexpected errors during transition: " + errors);
258+
259+
// Owner-thread tags should all be present — they were written before any contention
260+
for (int i = 0; i < 100; i++) {
261+
assertEquals("val_" + i, span.getTag("owner_" + i), "Owner tag owner_" + i + " missing");
262+
}
263+
264+
// Each thread's last write should be visible (earlier writes may be overwritten by races)
265+
for (int t = 0; t < numThreads; t++) {
266+
assertNotNull(
267+
span.getTag("thread" + t + "_" + (tagsPerThread - 1)),
268+
"Thread " + t + " last tag missing");
269+
}
270+
}
271+
272+
/**
273+
* Exercises many short-lived spans created on one thread and tagged from another — the exact
274+
* pattern the crossThread benchmark was trying to measure. Uses a stable handoff (CountDownLatch)
275+
* instead of the racy JMH setup.
276+
*/
277+
@Test
278+
@DisplayName("Many short spans tagged cross-thread — no NPE or crash")
279+
void manySpansCrossThread() throws Exception {
280+
int numSpans = 10_000;
281+
ExecutorService tagger = Executors.newFixedThreadPool(8);
282+
CopyOnWriteArrayList<Throwable> errors = new CopyOnWriteArrayList<>();
283+
284+
for (int s = 0; s < numSpans; s++) {
285+
AgentSpan span = TRACER.startSpan("test", "manyShort");
286+
CountDownLatch tagged = new CountDownLatch(8);
287+
288+
for (int t = 0; t < 8; t++) {
289+
tagger.submit(
290+
() -> {
291+
try {
292+
span.setTag("key", "value");
293+
} catch (Throwable e) {
294+
errors.add(e);
295+
} finally {
296+
tagged.countDown();
297+
}
298+
});
299+
}
300+
301+
tagged.await(5, TimeUnit.SECONDS);
302+
span.finish();
303+
}
304+
305+
tagger.shutdown();
306+
tagger.awaitTermination(10, TimeUnit.SECONDS);
307+
assertEquals(0, errors.size(), "Errors during cross-thread tagging of short spans: " + errors);
308+
}
162309
}

0 commit comments

Comments
 (0)