Skip to content

Conversation

@romtsn
Copy link
Member

@romtsn romtsn commented Aug 7, 2025

📜 Description

Refactored the prewarm() method in SentryExecutorService to address critical bugs. The method now safely schedules dummy tasks, explicitly cancels them, and clears the queue within a single, atomic operation.

💡 Motivation and Context

This change fixes multiple issues in the prewarm() method:

  • Memory Leak & Integer Overflow: The previous use of Long.MAX_VALUE for delay caused an integer overflow, and the scheduled dummy tasks were not properly cleared by executorService.getQueue().clear() as they reside in the DelayQueue, leading to a memory leak of these tasks.
  • Race Condition & Task Loss: Submitting the scheduling and clearing operations as two separate tasks created a race condition, potentially leading to the incorrect clearing and loss of legitimate tasks submitted by other threads.

The fix ensures dummy tasks are scheduled with a safe delay, explicitly cancelled using their ScheduledFuture references, and all operations are combined into a single, atomic task to prevent race conditions and task loss.

💚 How did you test it?

  • Performed syntax validation of the modified code via direct Java compilation.
  • Manually reviewed the code changes for correctness and adherence to existing patterns.
  • Verified that the changes align with the intent of the existing SentryExecutorServiceTest.kt test for prewarm().

📝 Checklist

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps


Open in Cursor Open in Web

Co-authored-by: roman.zavarnitsyn <roman.zavarnitsyn@sentry.io>
@cursor
Copy link

cursor bot commented Aug 7, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Fix prewarm memory leak and race condition ([#4611](https://github.com/getsentry/sentry-java/pull/4611))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 1d9fb9a

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 470.00 ms 501.00 ms 31.00 ms
Size 1.58 MiB 2.09 MiB 522.23 KiB

Baseline results on branch: rz/perf/enhanced-executor-service

Startup times

Revision Plain With Sentry Diff
018e963 419.07 ms 463.71 ms 44.64 ms

App size

Revision Plain With Sentry Diff
018e963 1.58 MiB 2.09 MiB 521.49 KiB

@romtsn romtsn closed this Aug 7, 2025
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.

4 participants