Skip to content

Fix error reporting in Schedulers#2036

Open
mikkolaj wants to merge 10 commits intomonix:mainfrom
AVSystem:fix-error-reporting-to-upstream
Open

Fix error reporting in Schedulers#2036
mikkolaj wants to merge 10 commits intomonix:mainfrom
AVSystem:fix-error-reporting-to-upstream

Conversation

@mikkolaj
Copy link
Copy Markdown

Original PR: AVSystem#15

Summary

In JDK 25 ForkJoinPool implements ScheduledExecutorService. This change resulted in incorrect error reporting and failing tests in AVSystem/monix/#6

Problem

Monix has two internal implementations of ExecutorScheduler:

  • FromSimpleExecutor - for regular ExecutorService
  • FromScheduledExecutor - for ScheduledExecutorService

The FromScheduledExecutor implementation relies on error reporting being done by the underlying executor itself (via afterExecute hook), but this only works correctly with AdaptedThreadPoolExecutor (Monix's custom implementation). For other ScheduledExecutorService implementations, error reporting did not work properly.

Before JDK 25 this problem wasn't visible, because almost all Scheduler factory methods use AsyncScheduler, FromSimpleExecutor or FromScheduledExecutor together with AdaptedThreadPoolExecutor.

Additionally, withUncaughtExceptionReporter didn't work correctly for all scheduler methods in wrapped schedulers (AsyncScheduler, etc.) - the updated reporter wasn't being used for scheduleOnce, scheduleAtFixedRate, and scheduleWithFixedDelay.

Changes

  1. Restricted FromScheduledExecutor (now FromAdaptedThreadPoolExecutor) usage - This implementation is now only used for schedulers backed by AdaptedScheduledThreadPoolExecutor. All other ExecutorService implementations (including ScheduledExecutorService and ForkJoinPool) use FromSimpleExecutor.

  2. Fixed error reporting in wrapped schedulers (ReferenceScheduler) - The scheduleOnce, scheduleAtFixedRate, and scheduleWithFixedDelay methods now wrap runnables with InterceptRunnable to pass exceptions to correct reporters.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants