NullPointerException in ScopeStack.checkOverdueScopes() due to race condition on 'top' field

[castor-d]

Tracer Version(s)

1.52.1

Java Version(s)

amazoncorretto:22-al2023 (docker base image)

JVM Vendor

Amazon Corretto

Bug Report

Note: This bug report was prepared with assistance from Claude AI for analysis and documentation.

A NullPointerException occurs in ScopeStack.checkOverdueScopes() method due to a race condition on the top field access.

Despite having a null check (if (top == null || top.source() != ITERATION)), the NPE still happens because the top field is not declared as volatile, allowing other threads to modify it between the null check and the subsequent method call.

Stack Trace

java.lang.NullPointerException: Cannot invoke "datadog.trace.agent.core.scopemanager.ContinuableScope.source()" because "this.top" is null
    at datadog.trace.agent.core.scopemanager.ScopeStack.checkOverdueScopes(ScopeStack.java:90)
    at datadog.trace.agent.core.scopemanager.ContinuableScope.close(ContinuableScope.java:50)
    at datadog.trace.instrumentation.springdata.RepositoryInterceptor.invoke(RepositoryInterceptor.java:48)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:223)
    at jdk.proxy3.$Proxy162.findAllActiveByPermissions
    at jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.lang.reflect.Method.invoke(Method.java:580)
    at kotlin.reflect.jvm.internal.calls.CallerImpl$Method.callMethod(CallerImpl.kt:97)
    at kotlin.reflect.jvm.internal.calls.CallerImpl$Method$Instance.call(CallerImpl.kt:113)
    at kotlin.reflect.jvm.internal.KCallableImpl.callDefaultMethod$kotlin_reflection(KCallableImpl.kt:207)
    at kotlin.reflect.full.KCallables.callSuspendBy(KCallables.kt:74)
    at org.springframework.core.CoroutinesUtils.lambda$invokeSuspendingFunction$3(CoroutinesUtils.java:143)
    at kotlin.coroutines.intrinsics.IntrinsicsKt__IntrinsicsJvmKt$createCoroutineUnintercepted$$inlined$createCoroutineFromSuspendFunction$IntrinsicsKt__IntrinsicsJvmKt$4.invokeSuspend(IntrinsicsJvm.kt:270)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at kotlinx.coroutines.EventLoop.processUnconfinedEvent(EventLoop.common.kt:65)
    at kotlinx.coroutines.DispatchedTaskKt.resumeUnconfined(DispatchedTask.kt:241)
    at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:159)
    at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:466)
    at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:500)
    at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:489)
    at kotlinx.coroutines.CancellableContinuationImpl.resumeWith(CancellableContinuationImpl.kt:364)
    at kotlinx.coroutines.reactor.MonoKt$awaitSingleOrNull$2$1.onComplete(Mono.kt:52)
    at reactor.core.publisher.StrictSubscriber.onComplete(StrictSubscriber.java:123)
    at reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber.onComplete(FluxContextWrite.java:126)
    at reactor.core.publisher.MonoNext$NextSubscriber.onComplete(MonoNext.java:102)
    at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:83)
    at reactor.core.publisher.FluxUsingWhen$UsingWhenSubscriber.onNext(FluxUsingWhen.java:348)
    at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
    at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
    at reactor.core.publisher.FluxFilter$FilterSubscriber.onNext(FluxFilter.java:113)
    at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82)
    at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79)
    at reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner.onNext(MonoFlatMapMany.java:251)
    at reactor.core.publisher.FluxDefaultIfEmpty$DefaultIfEmptySubscriber.onNext(FluxDefaultIfEmpty.java:122)
    at reactor.core.publisher.FluxMap$MapSubscriber.onNext(FluxMap.java:122)
    at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82)
    at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82)
    at io.lettuce.core.RedisPublisher$ImmediateSubscriber.onNext(RedisPublisher.java:895)
    at io.lettuce.core.RedisPublisher$RedisSubscription.onNext(RedisPublisher.java:295)
    at io.lettuce.core.RedisPublisher$SubscriptionCommand.doOnComplete(RedisPublisher.java:782)
    at io.lettuce.core.protocol.CommandWrapper.complete(CommandWrapper.java:69)
    at io.lettuce.core.protocol.CommandWrapper.complete(CommandWrapper.java:67)
    at io.lettuce.core.protocol.CommandHandler.complete(CommandHandler.java:769)
    at io.lettuce.core.protocol.CommandHandler.decode(CommandHandler.java:704)
    at io.lettuce.core.protocol.CommandHandler.channelRead(CommandHandler.java:621)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1519)
    at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1377)
    at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1428)
    at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:530)
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:469)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868)
    at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:799)
    at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:501)
    at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:399)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.lang.Thread.run(Thread.java:1570)

Root Cause

The issue occurs due to a race condition in multi-threaded environments:

1. Thread A evaluates the null check and finds top is not null
2. Thread B concurrently executes cleanup() method and sets top = null
3. Thread A then calls top.source() on a now-null reference
4. NPE is thrown

Impact

This race condition can cause unexpected crashes in applications using dd-trace-java in multi-threaded scenarios, particularly when scope management operations occur concurrently.

Expected Behavior

Expected Behavior

The checkOverdueScopes() method should handle concurrent access safely without throwing NullPointerException. The method should either:

1. Successfully complete the null check and proceed: When top is not null and is an ITERATION scope, the method should safely access top.source() and continue processing overdue scopes.

2. Return false safely: When top is null or not an ITERATION scope, the method should return false without any exceptions.

3. Maintain thread safety: The method should work correctly in multi-threaded environments where multiple threads may be concurrently modifying the scope stack through operations like cleanup(), push(), or scope closure.

The scope management should be robust and not crash the application due to race conditions between threads performing legitimate scope operations.

Reproduction Code

The race condition is difficult to reproduce consistently due to its timing-sensitive nature, but here's a test case that can help trigger the issue:

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;

public class ScopeStackRaceConditionTest {
    
    @Test
    public void testConcurrentScopeOperations() throws InterruptedException {
        final int THREAD_COUNT = 100;
        final int ITERATIONS = 1000;
        final CountDownLatch startLatch = new CountDownLatch(1);
        final CountDownLatch doneLatch = new CountDownLatch(THREAD_COUNT);
        final AtomicInteger exceptionCount = new AtomicInteger(0);
        
        ContinuableScopeManager scopeManager = new ContinuableScopeManager();
        ExecutorService executor = Executors.newFixedThreadPool(THREAD_COUNT);
        
        // Create multiple threads that concurrently manipulate scopes
        for (int i = 0; i < THREAD_COUNT; i++) {
            executor.submit(() -> {
                try {
                    startLatch.await();
                    
                    for (int j = 0; j < ITERATIONS; j++) {
                        try {
                            // Create and close scopes rapidly
                            AgentSpan span = GlobalTracer.get().buildSpan("test-span").start();
                            ContinuableScope scope = (ContinuableScope) scopeManager.activate(span);
                            
                            // This can trigger the race condition
                            scope.close();
                            
                        } catch (NullPointerException e) {
                            if (e.getMessage() != null && 
                                e.getMessage().contains("Cannot invoke") && 
                                e.getMessage().contains("source()") &&
                                e.getMessage().contains("this.top")) {
                                exceptionCount.incrementAndGet();
                            }
                        }
                    }
                } catch (InterruptedException e) {
                    Thread.currentThread().interrupt();
                } finally {
                    doneLatch.countDown();
                }
            });
        }
        
        // Start all threads simultaneously
        startLatch.countDown();
        doneLatch.await();
        executor.shutdown();
        
        // The NPE should be reproduced if the race condition occurs
        System.out.println("NPE occurrences: " + exceptionCount.get());
    }
}

Concurrent scope operations that can trigger the race condition

public void triggerRaceCondition() {
    ContinuableScopeManager scopeManager = new ContinuableScopeManager();
    
    // Thread 1: Continuously create and close scopes
    Thread scopeThread = new Thread(() -> {
        for (int i = 0; i < 10000; i++) {
            AgentSpan span = GlobalTracer.get().buildSpan("iteration-" + i).start();
            ContinuableScope scope = (ContinuableScope) scopeManager.activate(span);
            scope.close(); // This calls checkOverdueScopes()
        }
    });
    
    // Thread 2: Continuously trigger cleanup
    Thread cleanupThread = new Thread(() -> {
        for (int i = 0; i < 10000; i++) {
            // Access scope stack to trigger potential cleanup
            scopeManager.activeScope();
        }
    });
    
    scopeThread.start();
    cleanupThread.start();
    
    // Wait for threads to complete
    try {
        scopeThread.join();
        cleanupThread.join();
    } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
    }
}

[mcculls]

Hi @castor-d - thanks for raising this, scope stacks are only supposed to be active on one thread at a time. They are stored in a thread-local, which is why top should not need to be volatile. It's possible that a scope has leaked between threads - I'll also look at your test case.

BTW the triggerRaceCondition method you gave above doesn't compile because ContinuableScopeManager does not have an activeScope method - was that method built against a different version?

[mcculls]

Also the test case you provided doesn't compile - ContinuableScopeManager doesn't have a no-arg constructor, and its activate method is private and is not meant to be called from outside of the class.

Could you supply a version of your testcase that will build against master ?

[mcculls]

Hi @castor-d - I looked at the original stack trace and have a potential lead.

This is specific to an edge-case in Kotlin coroutines and is not a general issue with the ScopeStack code.

( Claude AI seems to have made assumptions that don't apply to how that code is used in practice. )

[mcculls]

I put together a potential fix in #9403 which will be in v1.53.0 - are you able to test a snapshot build?

[castor-d]

Hi @mcculls, I have to mention that I asked a cloud AI to help me write the original issue description since my English is not very good.
Also, I wasn’t able to actually run the provided test code to verify it — sorry about that.

This is also the first time I’ve encountered such a case in several years of using dd-trace, so I think it will be hard for me to reproduce it consistently.
Due to some special circumstances in our company, we cannot use version management tools like Maven, so we are directly using the built JAR files by placing them in our project.

Once the official v1.53.0 release is built and published, I will test it on our side.

Thank you.

[castor-d]

Hi @mcculls

I downloaded and applied version v1.53.0 today, but it caused an NPE like the one below.
Looking at your PR( #9403 ), it seems to be a fix for ScopeStack, so it feels somehow related.
Could you check it out?

NullPointerException

java.util.ArrayDeque in addLast at line 302
java.util.ArrayDeque in forEach at line 887
java.util.ArrayDeque in copyElements at line 328
java.util.ArrayDeque in addAll at line 323
datadog.trace.agent.core.scopemanager.ScopeStack in copy at line 29
datadog.trace.agent.core.scopemanager.ScopeContext in restore at line 24
datadog.trace.agent.core.scopemanager.ContinuableScopeManager in swap at line 370
datadog.context.Context in swap at line 72
datadog.trace.instrumentation.kotlin.coroutines.DatadogThreadContextElement in updateThreadContext at line 60
datadog.trace.instrumentation.kotlin.coroutines.DatadogThreadContextElement in updateThreadContext at line 14
kotlinx.coroutines.internal.ThreadContextKt in updateThreadContext at line 74
kotlinx.coroutines.DispatchedTask in run at line 221
kotlinx.coroutines.EventLoop in processUnconfinedEvent at line 65
kotlinx.coroutines.DispatchedTaskKt in resumeUnconfined at line 241
kotlinx.coroutines.DispatchedTaskKt in dispatch at line 159
kotlinx.coroutines.CancellableContinuationImpl in dispatchResume at line 466
kotlinx.coroutines.CancellableContinuationImpl in resumeImpl at line 500
kotlinx.coroutines.CancellableContinuationImpl in resumeImpl$default at line 489
kotlinx.coroutines.CancellableContinuationImpl in resumeWith at line 364
kotlinx.coroutines.reactor.MonoKt$awaitSingleOrNull$2$1 in onComplete at line 52
reactor.core.publisher.StrictSubscriber in onComplete at line 123
reactor.core.publisher.FluxContextWrite$ContextWriteSubscriber in onComplete at line 126
reactor.core.publisher.MonoNext$NextSubscriber in onComplete at line 102
reactor.core.publisher.MonoNext$NextSubscriber in onNext at line 83
reactor.core.publisher.FluxUsingWhen$UsingWhenSubscriber in onNext at line 348
reactor.core.publisher.FluxMap$MapSubscriber in onNext at line 122
reactor.core.publisher.FluxFilter$FilterSubscriber in onNext at line 113
reactor.core.publisher.MonoNext$NextSubscriber in onNext at line 82
reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber in onNext at line 79
reactor.core.publisher.MonoFlatMapMany$FlatMapManyInner in onNext at line 251
reactor.core.publisher.FluxDefaultIfEmpty$DefaultIfEmptySubscriber in onNext at line 122
reactor.core.publisher.FluxMap$MapSubscriber in onNext at line 122
reactor.core.publisher.MonoNext$NextSubscriber in onNext at line 82
io.lettuce.core.RedisPublisher$ImmediateSubscriber in onNext at line 895
io.lettuce.core.RedisPublisher$RedisSubscription in onNext at line 295
io.lettuce.core.RedisPublisher$SubscriptionCommand in doOnComplete at line 782
io.lettuce.core.protocol.CommandWrapper in complete at line 69
io.lettuce.core.protocol.CommandWrapper in complete at line 67
io.lettuce.core.protocol.CommandHandler in complete at line 769
io.lettuce.core.protocol.CommandHandler in decode at line 704
io.lettuce.core.protocol.CommandHandler in channelRead at line 621
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 442
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 420
io.netty.channel.AbstractChannelHandlerContext in fireChannelRead at line 412
io.netty.handler.ssl.SslHandler in unwrap at line 1519
io.netty.handler.ssl.SslHandler in decodeJdkCompatible at line 1377
io.netty.handler.ssl.SslHandler in decode at line 1428
io.netty.handler.codec.ByteToMessageDecoder in decodeRemovalReentryProtection at line 530
io.netty.handler.codec.ByteToMessageDecoder in callDecode at line 469
io.netty.handler.codec.ByteToMessageDecoder in channelRead at line 290
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 444
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 420
io.netty.channel.AbstractChannelHandlerContext in fireChannelRead at line 412
io.netty.channel.DefaultChannelPipeline$HeadContext in channelRead at line 1357
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 440
io.netty.channel.AbstractChannelHandlerContext in invokeChannelRead at line 420
io.netty.channel.DefaultChannelPipeline in fireChannelRead at line 868
io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe in epollInReady at line 799
io.netty.channel.epoll.EpollEventLoop in processReady at line 501
io.netty.channel.epoll.EpollEventLoop in run at line 399
io.netty.util.concurrent.SingleThreadEventExecutor$4 in run at line 998
io.netty.util.internal.ThreadExecutorMap$2 in run at line 74
io.netty.util.concurrent.FastThreadLocalRunnable in run at line 30
java.lang.Thread in run at line 1570

[mcculls]

Hi @castor-d this indicates that Kotlin coroutines is for some reason restoring the same cached context on two separate threads.

The first time it restores it on the original thread, which we don't need to copy because it is the same thread.
That original thread then proceeds to work on the restored stack.

Some time later Kotlin coroutines restores the same context on a different thread - we attempt to take a defensive copy, but the original thread has begun to mutate the context.

We still haven't been able to recreate this locally, so we'll need to add another defensive change.

[mcculls]

@castor-d also you mentioned:

we are directly using the built JAR files by placing them in our project

does this mean that if I attach a JAR with the new fix you would be able to test it?

[mcculls]

Here's a JAR built from #9491

https://s3.us-east-1.amazonaws.com/dd-trace-java-builds/75804759/dd-java-agent.jar

it would be helpful if you could try it out and report back - thanks in advance!

[castor-d]

@mcculls Yes, we can use the agent file you provided because we download the jar file directly and then use it inside a Docker container image.
The NPE mentioned above is also not something we can arbitrarily trigger, so after applying the file, we can only determine if errors occur through monitoring.
We plan to download it today and apply it to the project. If there are no error reports from my end for a certain period afterward, you can consider the issue resolved.

[mcculls]

Thanks - will keep an eye out for updates

[mcculls]

Hi @castor-d, after additional local testing I made some further improvements.

The new snapshot is available from https://s3.us-east-1.amazonaws.com/dd-trace-java-builds/76415451/dd-java-agent.jar

The git-hash of this new snapshot, reported by java -jar dd-java-agent.jar, should be 526162b

[castor-d]

@mcculls Thank you for letting me know. The version applied last week did not cause any issues.

I'll replace it again with the jar file you mentioned in the comment and try running it.

It seems like version 1.54.0 is scheduled for release.
Is there any way to know when the planned release date is?