From d2580d7682e1258ea1d1c5197f99410a84603867 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 30 Jul 2026 13:40:54 +0200 Subject: [PATCH 1/5] fix(core): Report no-op executor tasks as cancelled (JAVA-628) NoOpSentryExecutorService returned a FutureTask that is never run and never cancelled, so a caller had no way to tell a dropped task from a queued one: isCancelled() and isDone() both stay false forever, and get() blocks until it times out. Return an already-cancelled Future instead, matching what SentryExecutorService already does when its work queue is full, and state the requirement on ISentryExecutorService so future implementations honour it. CancelledFuture moves out of SentryExecutorService so both services share one implementation. An ActivityLifecycleIntegration test asserted a freshly scheduled ttfd timeout was not cancelled while running on the no-op executor, so it was really pinning the old pending-forever behaviour rather than the scheduling it names. It now uses DeferredExecutorService, which queues. Co-Authored-By: Claude Opus 5 (1M context) --- .../core/ActivityLifecycleIntegrationTest.kt | 2 + .../src/main/kotlin/io/sentry/test/Mocks.kt | 11 ++++-- .../main/java/io/sentry/CancelledFuture.java | 38 +++++++++++++++++++ .../io/sentry/ISentryExecutorService.java | 21 ++++++++-- .../io/sentry/NoOpSentryExecutorService.java | 7 ++-- .../java/io/sentry/SentryExecutorService.java | 28 -------------- .../sentry/NoOpSentryExecutorServiceTest.kt | 15 ++++++++ 7 files changed, 83 insertions(+), 39 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/CancelledFuture.java diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 166129f601b..c79d418efe5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -1975,6 +1975,8 @@ class ActivityLifecycleIntegrationTest { val sut = fixture.getSut() fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true + // the timeout has to be really scheduled for cancelling it to be observable + fixture.options.executorService = DeferredExecutorService() sut.register(fixture.scopes, fixture.options) val activity = mock() val activity2 = mock() diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt index 79076e9dc8f..b66c0f29475 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt @@ -76,12 +76,15 @@ class DeferredExecutorService : ISentryExecutorService { } class NonOverridableNoOpSentryExecutorService : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = FutureTask { null } + // mirrors NoOpSentryExecutorService: a task that is never run is reported as cancelled, so + // callers can tell it apart from a queued one + private fun cancelledFuture(): FutureTask = FutureTask { null }.apply { cancel(false) } - override fun submit(callable: Callable): Future = FutureTask { null } + override fun submit(runnable: Runnable): Future<*> = cancelledFuture() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = - FutureTask { null } + override fun submit(callable: Callable): Future = cancelledFuture() + + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = cancelledFuture() override fun close(timeoutMillis: Long) {} diff --git a/sentry/src/main/java/io/sentry/CancelledFuture.java b/sentry/src/main/java/io/sentry/CancelledFuture.java new file mode 100644 index 00000000000..502651597cb --- /dev/null +++ b/sentry/src/main/java/io/sentry/CancelledFuture.java @@ -0,0 +1,38 @@ +package io.sentry; + +import java.util.concurrent.CancellationException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import org.jetbrains.annotations.NotNull; + +/** + * The {@link Future} an {@link ISentryExecutorService} hands back for a task it will never run. + * Callers can tell such a task apart from an accepted one through {@link #isCancelled()}, and are + * spared blocking in {@link #get()} on a result that is never coming. + */ +final class CancelledFuture implements Future { + @Override + public boolean cancel(final boolean mayInterruptIfRunning) { + return true; + } + + @Override + public boolean isCancelled() { + return true; + } + + @Override + public boolean isDone() { + return true; + } + + @Override + public T get() { + throw new CancellationException(); + } + + @Override + public T get(final long timeout, final @NotNull TimeUnit unit) { + throw new CancellationException(); + } +} diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 9bdef8db2b7..0152d227a3c 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -6,7 +6,15 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; -/** Sentry Executor Service that sends cached events and envelopes on App. start. */ +/** + * Sentry Executor Service that sends cached events and envelopes on App. start. + * + *

Implementations that drop a task instead of running it — a no-op service, a full work queue — + * must report that through the returned {@link Future}: either throw {@link + * RejectedExecutionException} or return a Future that is already cancelled. Callers rely on this to + * tell a queued task from a dropped one, so a Future that will never complete and never reports + * cancellation leaves them waiting on a task that is never coming. + */ @ApiStatus.Internal public interface ISentryExecutorService { @@ -14,7 +22,7 @@ public interface ISentryExecutorService { * Submits a Runnable to the ThreadExecutor * * @param runnable the Runnable - * @return a Future of the Runnable + * @return a Future of the Runnable, already cancelled if the task will not be run */ @NotNull Future submit(final @NotNull Runnable runnable) throws RejectedExecutionException; @@ -23,11 +31,18 @@ public interface ISentryExecutorService { * Submits a Callable to the ThreadExecutor * * @param callable the Callable - * @return a Future of the Callable + * @return a Future of the Callable, already cancelled if the task will not be run */ @NotNull Future submit(final @NotNull Callable callable) throws RejectedExecutionException; + /** + * Schedules a Runnable on the ThreadExecutor + * + * @param runnable the Runnable + * @param delayMillis how long to wait before running it + * @return a Future of the Runnable, already cancelled if the task will not be run + */ @NotNull Future schedule(final @NotNull Runnable runnable, final long delayMillis) throws RejectedExecutionException; diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index 37778c3cc09..b0ed80a86a1 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -2,7 +2,6 @@ import java.util.concurrent.Callable; import java.util.concurrent.Future; -import java.util.concurrent.FutureTask; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -18,17 +17,17 @@ private NoOpSentryExecutorService() {} @Override public @NotNull Future submit(final @NotNull Runnable runnable) { - return new FutureTask<>(() -> null); + return new CancelledFuture<>(); } @Override public @NotNull Future submit(final @NotNull Callable callable) { - return new FutureTask<>(() -> null); + return new CancelledFuture<>(); } @Override public @NotNull Future schedule(@NotNull Runnable runnable, long delayMillis) { - return new FutureTask<>(() -> null); + return new CancelledFuture<>(); } @Override diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index a469dca5853..e20c0835637 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -2,7 +2,6 @@ import io.sentry.util.AutoClosableReentrantLock; import java.util.concurrent.Callable; -import java.util.concurrent.CancellationException; import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -155,31 +154,4 @@ private static final class SentryExecutorServiceThread extends Thread { super(r, name); } } - - private static final class CancelledFuture implements Future { - @Override - public boolean cancel(final boolean mayInterruptIfRunning) { - return true; - } - - @Override - public boolean isCancelled() { - return true; - } - - @Override - public boolean isDone() { - return true; - } - - @Override - public T get() { - throw new CancellationException(); - } - - @Override - public T get(final long timeout, final @NotNull TimeUnit unit) { - throw new CancellationException(); - } - } } diff --git a/sentry/src/test/java/io/sentry/NoOpSentryExecutorServiceTest.kt b/sentry/src/test/java/io/sentry/NoOpSentryExecutorServiceTest.kt index f9df0c52dda..212c1dcde56 100644 --- a/sentry/src/test/java/io/sentry/NoOpSentryExecutorServiceTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpSentryExecutorServiceTest.kt @@ -1,7 +1,10 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import java.util.concurrent.Callable +import java.util.concurrent.CancellationException import kotlin.test.Test +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull import org.mockito.kotlin.mock @@ -27,6 +30,18 @@ class NoOpSentryExecutorServiceTest { assertNotNull(future) } + @Test + fun `submitted tasks are reported as cancelled instead of pending forever`() { + assertThat(sut.submit(mock()).isCancelled).isTrue() + assertThat(sut.submit(mock>()).isCancelled).isTrue() + assertThat(sut.schedule(mock(), 0).isCancelled).isTrue() + } + + @Test + fun `getting the result of a dropped task fails fast`() { + assertFailsWith { sut.submit(mock()).get() } + } + @Test fun `close does not throw`() = sut.close(0) @Test From 63af68f860996e8eb829c11ed2b9871e4704d905 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 30 Jul 2026 13:41:44 +0200 Subject: [PATCH 2/5] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89d412bcfdd..c330ef84e7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Avoid a CPU busy-loop when recording discarded log or metric envelopes under rate limiting ([#5835](https://github.com/getsentry/sentry-java/pull/5835)) - `ClientReportRecorder` now reads the item count from the envelope item header instead of deserializing the payload, which under sustained rate limiting could pin CPU cores while repeatedly throwing exceptions +- Report tasks handed to a no-op `ISentryExecutorService` as cancelled ([#5874](https://github.com/getsentry/sentry-java/pull/5874)) + - `NoOpSentryExecutorService` returned a `Future` that was never run and never cancelled, so callers could not tell a dropped task from a queued one and `get()` would block until its timeout ### Performance From 81da6abbc9fc4c413490f2d41490f0ef7a6ef0f8 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 30 Jul 2026 14:44:38 +0200 Subject: [PATCH 3/5] Update sentry/src/main/java/io/sentry/ISentryExecutorService.java Co-authored-by: arb --- sentry/src/main/java/io/sentry/ISentryExecutorService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 0152d227a3c..6f4a2af0ee6 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -7,7 +7,7 @@ import org.jetbrains.annotations.NotNull; /** - * Sentry Executor Service that sends cached events and envelopes on App. start. + * Sentry Executor Service that sends cached events and envelopes on App start. * *

Implementations that drop a task instead of running it — a no-op service, a full work queue — * must report that through the returned {@link Future}: either throw {@link From 7125c462e5fbb85dfe74d5f3686ebc58c240799e Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Thu, 30 Jul 2026 14:45:01 +0200 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: arb --- CHANGELOG.md | 2 +- sentry/src/main/java/io/sentry/ISentryExecutorService.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c330ef84e7f..226c7cfb54f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ - Avoid a CPU busy-loop when recording discarded log or metric envelopes under rate limiting ([#5835](https://github.com/getsentry/sentry-java/pull/5835)) - `ClientReportRecorder` now reads the item count from the envelope item header instead of deserializing the payload, which under sustained rate limiting could pin CPU cores while repeatedly throwing exceptions - Report tasks handed to a no-op `ISentryExecutorService` as cancelled ([#5874](https://github.com/getsentry/sentry-java/pull/5874)) - - `NoOpSentryExecutorService` returned a `Future` that was never run and never cancelled, so callers could not tell a dropped task from a queued one and `get()` would block until its timeout + - `NoOpSentryExecutorService` previously returned a `Future` that was never run and never cancelled, so callers could not tell a dropped task from a queued one and `get()` would block until its timeout ### Performance diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 6f4a2af0ee6..4e5b16876b3 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -11,7 +11,7 @@ * *

Implementations that drop a task instead of running it — a no-op service, a full work queue — * must report that through the returned {@link Future}: either throw {@link - * RejectedExecutionException} or return a Future that is already cancelled. Callers rely on this to + * RejectedExecutionException} or return a {@link CancelledFuture}. Callers rely on this to * tell a queued task from a dropped one, so a Future that will never complete and never reports * cancellation leaves them waiting on a task that is never coming. */ From 139e64c80b5fdb4f7fa470835b5a4e832c69b8b9 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 30 Jul 2026 12:47:42 +0000 Subject: [PATCH 5/5] Format code --- sentry/src/main/java/io/sentry/ISentryExecutorService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 4e5b16876b3..656308dae4a 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -11,8 +11,8 @@ * *

Implementations that drop a task instead of running it — a no-op service, a full work queue — * must report that through the returned {@link Future}: either throw {@link - * RejectedExecutionException} or return a {@link CancelledFuture}. Callers rely on this to - * tell a queued task from a dropped one, so a Future that will never complete and never reports + * RejectedExecutionException} or return a {@link CancelledFuture}. Callers rely on this to tell a + * queued task from a dropped one, so a Future that will never complete and never reports * cancellation leaves them waiting on a task that is never coming. */ @ApiStatus.Internal