From 1d9fb9a1f5bc53259357d4fa4fb40034fd390270 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 7 Aug 2025 07:07:38 +0000 Subject: [PATCH 1/2] Improve SentryExecutorService prewarm method with safer task scheduling Co-authored-by: roman.zavarnitsyn --- .../java/io/sentry/SentryExecutorService.java | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index c7700ee36bd..4e94b0aeed8 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -1,9 +1,12 @@ package io.sentry; import io.sentry.util.AutoClosableReentrantLock; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.Future; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; @@ -123,17 +126,34 @@ public boolean isClosed() { public void prewarm() { executorService.submit( () -> { - // schedule a bunch of dummy runnables in the future that will never execute to trigger - // queue - // growth and then clear the queue up - for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { - executorService.schedule(dummyRunnable, Long.MAX_VALUE, TimeUnit.DAYS); + // Schedule dummy tasks with a reasonable delay to trigger queue growth + // Use a delay that won't cause integer overflow when converted to nanoseconds + // 365 days is safe (Long.MAX_VALUE / TimeUnit.DAYS.toNanos(1) > 365) + final long safeDelayDays = 365L; + + // Store references to the scheduled futures so we can cancel them properly + // since scheduled tasks are stored in DelayQueue, not the main work queue + final List> scheduledTasks = new ArrayList<>(INITIAL_QUEUE_SIZE); + + try { + // Schedule dummy tasks to trigger queue growth + for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { + ScheduledFuture future = + executorService.schedule(dummyRunnable, safeDelayDays, TimeUnit.DAYS); + scheduledTasks.add(future); + } + } finally { + // Cancel all scheduled dummy tasks to prevent memory leak + // This must be done in the same task to avoid race conditions + for (ScheduledFuture future : scheduledTasks) { + future.cancel(false); + } + + // Clear any remaining tasks from the main work queue + // This is safe now since we're in a single task execution + executorService.getQueue().clear(); } }); - executorService.submit( - () -> { - executorService.getQueue().clear(); - }); } private static final class SentryExecutorServiceThreadFactory implements ThreadFactory { From 9246089160dc48916d98773d8260cc71425865f3 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 7 Aug 2025 07:10:54 +0000 Subject: [PATCH 2/2] Format code --- sentry/src/main/java/io/sentry/SentryExecutorService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index 4e94b0aeed8..2ae45004b5f 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -130,15 +130,15 @@ public void prewarm() { // Use a delay that won't cause integer overflow when converted to nanoseconds // 365 days is safe (Long.MAX_VALUE / TimeUnit.DAYS.toNanos(1) > 365) final long safeDelayDays = 365L; - + // Store references to the scheduled futures so we can cancel them properly // since scheduled tasks are stored in DelayQueue, not the main work queue final List> scheduledTasks = new ArrayList<>(INITIAL_QUEUE_SIZE); - + try { // Schedule dummy tasks to trigger queue growth for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) { - ScheduledFuture future = + ScheduledFuture future = executorService.schedule(dummyRunnable, safeDelayDays, TimeUnit.DAYS); scheduledTasks.add(future); } @@ -148,7 +148,7 @@ public void prewarm() { for (ScheduledFuture future : scheduledTasks) { future.cancel(false); } - + // Clear any remaining tasks from the main work queue // This is safe now since we're in a single task execution executorService.getQueue().clear();