java.util.concurrent.RejectedExecutionException

[markushi]

Integration

sentry-android

Build System

Gradle

AGP Version

unkown

Proguard

Enabled

Version

6.9.2

Steps to Reproduce

Reported by a customer.

Right now Sentry.close() closes the hub, which itself calls options.getExecutorService().close(). If someone would re-use the same options to initialize the SDK again, it would crash.

val options = SentryOptions()
Sentry.init(options)
Sentry.close()
Sentry.init(options) // executor service is shutdown at this point

Expected Result

SDK does not crash.

Actual Result

Stack Trace

Fatal Exception: java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask@3694f95 rejected from java.util.concurrent.ScheduledThreadPoolExecutor@c5478aa[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 17]
       at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2086)
       at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:848)
       at java.util.concurrent.ScheduledThreadPoolExecutor.delayedExecute(ScheduledThreadPoolExecutor.java:334)
       at java.util.concurrent.ScheduledThreadPoolExecutor.schedule(ScheduledThreadPoolExecutor.java:562)
       at java.util.concurrent.ScheduledThreadPoolExecutor.submit(ScheduledThreadPoolExecutor.java:664)
       at java.util.concurrent.Executors$DelegatedExecutorService.submit(Executors.java:640)
       at io.sentry.SentryExecutorService.submit(SentryExecutorService.java:25)
       at io.sentry.android.core.AndroidTransactionProfiler.onTransactionFinish(AndroidTransactionProfiler.java:246)
       at io.sentry.SentryTracer.finish(SentryTracer.java:331)
       at io.sentry.SentryTracer.finish(SentryTracer.java:321)
       at io.sentry.SentryTracer$1.run(SentryTracer.java:147)
       at java.util.TimerThread.mainLoop(TimerThread.java:562)
       at java.util.TimerThread.run(TimerThread.java:512)

[adinauer]

I guess simply creating the executor service lazily (having it null initially and resetting it to null on close) would only make us run into the next similar problem. I guess SentryClient and integrations would be the next problematic things.

Should we instead check on init if the options have already gone through a close cycle and reject them to at least give a meaningful error message?

In addition we could optionally offer a copy constructor for SentryOptions.

[markushi]

Yes, I agree - just fixing the ExecuterService is not enough and it will likely fail somewhere else instead.

I like the idea of checking the options 👍
IMHO we should still check all calls to executerService.submit() and ensure they handle potential exceptions as some async (system) callbacks might trigger a call to the SDK although Sentry.close() has been called already.

[romtsn]

To add to that^, we should probably do 2 things:

• Have a submit and schedule methods wrapped into try-catch, because the SDK should not crash if a background task cannot be executed (we do not have any critical codepaths that are using submit/schedule)
• Call Scope.clear() inside the Hub.close() method to free the memory up. Scope.clear() should also ensure there's no pending callbacks are left when Hub is closed (e.g. call ISpan.cancelFinishTask or something alike for all children of the transaction on scope)