| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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) <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
There was a problem hiding this comment.
Nice! A few optional nits; otherwise lgtm pending fix of the SentryBot issue.
Sorry, something went wrong.
Co-authored-by: arb <adam.brown@sentry.io>
Co-authored-by: arb <adam.brown@sentry.io>
Performance metrics 🚀
Baseline results on branch: mainStartup times
App size
Previous results on branch: no/noop-executor-cancelled-futureStartup times
App size
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
📜 Description
NoOpSentryExecutorService returned new FutureTask<>(() -> null) from submit/schedule — a task that is never run and never cancelled. Callers had no way to distinguish it from a genuinely queued task: isCancelled() and isDone() both stay false forever, and get() blocks until its timeout.
It now returns an already-cancelled Future, matching what SentryExecutorService already does when its work queue is full. CancelledFuture moves out of SentryExecutorService into a shared package-private class, and ISentryExecutorService documents the requirement so future implementations honour it.
No public API change — CancelledFuture is package-private and no existing signature changed shape, so .api files are untouched.
💡 Motivation and Context
Found by cursor here. That PR's PersistingScopeObserver uses future.isCancelled() to detect a flush task the executor dropped, and releases its hasPendingFlush latch when it sees one. Against a no-op executor the latch would be set with no task to ever clear it, silently stopping scope persistence for the rest of the process.
That is not a reachable bug today because the observer is only installed after options.activate() has swapped in a real executor so this more to prevent regressions in the future. But setExecutorService is public, and the contract the caller relies on was never written down anywhere. Fixing it here, off main, keeps #5791 focused and makes the guarantee available to any other caller that wants to check whether its task was accepted.
💚 How did you test it?
Tests pass
📝 Checklist
🔮 Next steps
#5791 builds on this guarantee for its scope-write batching.