Skip to content

CompletableResultCode completion callbacks can deadlock or prevent aggregate completion #8771

Description

@johnbley

Describe the bug

CompletableResultCode invokes completion callbacks while holding the result's internal lock. Two
results with callbacks that complete each other can therefore deadlock through lock inversion.

Additionally, if one callback throws, callback iteration stops. A later callback is not invoked; in
particular, an ofAll callback can be skipped, leaving the aggregate incomplete indefinitely.

Steps to reproduce

For the deadlock:

CompletableResultCode first = new CompletableResultCode();
CompletableResultCode second = new CompletableResultCode();
CountDownLatch callbacksEntered = new CountDownLatch(2);
CountDownLatch release = new CountDownLatch(1);

first.whenComplete(() -> {
  callbacksEntered.countDown();
  awaitUninterruptibly(release);
  second.succeed();
});
second.whenComplete(() -> {
  callbacksEntered.countDown();
  awaitUninterruptibly(release);
  first.succeed();
});

Thread firstThread = new Thread(first::succeed);
Thread secondThread = new Thread(second::succeed);
firstThread.setDaemon(true);
secondThread.setDaemon(true);
firstThread.start();
secondThread.start();
assert callbacksEntered.await(3, TimeUnit.SECONDS);
release.countDown();
firstThread.join(3_000);
secondThread.join(3_000);

Both callbacks are guaranteed to be running before either tries to complete the other result. The
threads can then wait forever for the other result's lock.

For the callback-abort case:

CompletableResultCode source = new CompletableResultCode();
CompletableResultCode other = new CompletableResultCode();
AtomicBoolean laterCallbackInvoked = new AtomicBoolean();

source.whenComplete(() -> {
  throw new RuntimeException("callback failure");
});

CompletableResultCode all = CompletableResultCode.ofAll(Arrays.asList(source, other));
source.whenComplete(() -> laterCallbackInvoked.set(true));

try {
  source.succeed();
} catch (RuntimeException ignored) {
  // The callback exception currently escapes completion.
}
other.succeed();

After this sequence, laterCallbackInvoked remains false and all.isDone() remains false
because the callback installed by ofAll and the later callback were never invoked.

What did you expect to see?

Completion callbacks should not cause cross-result lock inversion, and one callback failure should
not prevent later callbacks from running or prevent an ofAll aggregate from reaching completion.

What did you see instead?

Completion threads can deadlock, and callback exceptions can leave later callbacks—and consequently
an ofAll result—permanently incomplete.

What version and what artifacts are you using?

Artifacts: opentelemetry-sdk-common

Version: 1.66.0-SNAPSHOT from the current repository checkout

How did you reference these artifacts? Source files under sdk/common/src/main.

Environment

Compiler: Not applicable; source-level issue

OS: Not applicable; source-level issue

Runtime: Not applicable; source-level issue

Additional context

The affected implementation is io.opentelemetry.sdk.common.CompletableResultCode. It models the
Java 8 CompletableFuture API, whose completion actions do not hold a result monitor while running
and whose dependent-action failures do not abort processing of unrelated completions.

Finally, this issue was found, test-confirmed, and documented by Codex. I, a human, have applied my judgement and believe this to be credible work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions