Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Probe class availability without initializing the class during SDK init ([#5635](https://github.com/getsentry/sentry-java/pull/5635))
- Avoid constructing an exception per view when resolving view ids during view-hierarchy and gesture capture ([#5631](https://github.com/getsentry/sentry-java/pull/5631))
- Start the frame metrics thread lazily on first collection instead of during SDK init ([#5641](https://github.com/getsentry/sentry-java/pull/5641))
- Schedule transaction idle/deadline timeouts on a shared, dedicated executor instead of spawning a `Timer` thread per transaction ([#5670](https://github.com/getsentry/sentry-java/pull/5670))

## 8.46.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.sentry.Scopes
import io.sentry.Sentry
import io.sentry.SentryDate
import io.sentry.SentryDateProvider
import io.sentry.SentryExecutorService
import io.sentry.SentryNanotimeDate
import io.sentry.SentryTraceHeader
import io.sentry.SentryTracer
Expand Down Expand Up @@ -654,6 +655,8 @@ class ActivityLifecycleIntegrationTest {
it.idleTimeout = 100
}
)
// the transaction idle timeout is scheduled on the dedicated timer executor
fixture.options.timerExecutorService = SentryExecutorService()
sut.register(fixture.scopes, fixture.options)
sut.onActivityCreated(activity, fixture.bundle)

Expand Down
2 changes: 2 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -3702,6 +3702,7 @@ public class io/sentry/SentryOptions {
public fun getSslSocketFactory ()Ljavax/net/ssl/SSLSocketFactory;
public fun getTags ()Ljava/util/Map;
public fun getThreadChecker ()Lio/sentry/util/thread/IThreadChecker;
public fun getTimerExecutorService ()Lio/sentry/ISentryExecutorService;
public fun getTracePropagationTargets ()Ljava/util/List;
public fun getTracesSampleRate ()Ljava/lang/Double;
public fun getTracesSampler ()Lio/sentry/SentryOptions$TracesSamplerCallback;
Expand Down Expand Up @@ -3866,6 +3867,7 @@ public class io/sentry/SentryOptions {
public fun setStrictTraceContinuation (Z)V
public fun setTag (Ljava/lang/String;Ljava/lang/String;)V
public fun setThreadChecker (Lio/sentry/util/thread/IThreadChecker;)V
public fun setTimerExecutorService (Lio/sentry/ISentryExecutorService;)V
public fun setTraceOptionsRequests (Z)V
public fun setTracePropagationTargets (Ljava/util/List;)V
public fun setTraceSampling (Z)V
Expand Down
1 change: 1 addition & 0 deletions sentry/src/main/java/io/sentry/Scopes.java
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ public void close(final boolean isRestarting) {
getOptions().getContinuousProfiler().close(true);
getOptions().getCompositePerformanceCollector().close();
getOptions().getConnectionStatusProvider().close();
getOptions().getTimerExecutorService().close(getOptions().getShutdownTimeoutMillis());
final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService();
if (isRestarting) {
try {
Expand Down
4 changes: 4 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,10 @@ private static void init(final @NotNull SentryOptions options, final boolean glo
options.getExecutorService().prewarm();
}

if (options.getTimerExecutorService().isClosed()) {
options.setTimerExecutorService(new SentryExecutorService(options, true));
}

// load lazy fields of the options in a separate thread
try {
options.getExecutorService().submit(() -> options.loadLazyFields());
Expand Down
7 changes: 7 additions & 0 deletions sentry/src/main/java/io/sentry/SentryExecutorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ public SentryExecutorService(final @Nullable SentryOptions options) {
this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), options);
}

SentryExecutorService(final @Nullable SentryOptions options, final boolean removeOnCancelPolicy) {
this(options);
// removes cancelled tasks from the work queue immediately instead of leaving them until their
// scheduled time; useful for executors that frequently reschedule (e.g. transaction timeouts)
executorService.setRemoveOnCancelPolicy(removeOnCancelPolicy);
}

public SentryExecutorService() {
this(new ScheduledThreadPoolExecutor(1, new SentryExecutorServiceThreadFactory()), null);
}
Expand Down
39 changes: 39 additions & 0 deletions sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,14 @@ public class SentryOptions {
/** Sentry Executor Service that sends cached events and envelopes on App. start. */
private @NotNull ISentryExecutorService executorService = NoOpSentryExecutorService.getInstance();

/**
* Dedicated executor for scheduling transaction idle/deadline timeouts. Kept separate from {@link
* #executorService} so timeout callbacks (which finish transactions) don't contend with cached
* event sending.
*/
private @NotNull ISentryExecutorService timerExecutorService =
NoOpSentryExecutorService.getInstance();

/**
* Whether SpotlightIntegration has already been loaded via reflection. This prevents re-adding it
* if the user removed it in their configuration callback and activate() is called again.
Expand Down Expand Up @@ -681,6 +689,13 @@ public void activate() {
executorService.prewarm();
}

if (timerExecutorService instanceof NoOpSentryExecutorService) {
// Not prewarmed: its single worker thread is spawned lazily on the first scheduled timeout
// and then reused across all transactions. removeOnCancelPolicy keeps the work queue from
// accumulating cancelled timeouts (idle timers are cancelled and rescheduled per child span).
timerExecutorService = new SentryExecutorService(this, true);
}

// SpotlightIntegration is loaded via reflection to allow the sentry-spotlight module
// to be excluded from release builds, preventing insecure HTTP URLs from appearing in APKs.
// Only attempt once to avoid re-adding after user removal in their configuration callback.
Expand Down Expand Up @@ -1568,6 +1583,30 @@ public void setExecutorService(final @NotNull ISentryExecutorService executorSer
}
}

/**
* Returns the dedicated executor used to schedule transaction idle/deadline timeouts.
*
* @return the timer executor service
*/
@ApiStatus.Internal
@NotNull
public ISentryExecutorService getTimerExecutorService() {
return timerExecutorService;
}

/**
* Sets the dedicated executor used to schedule transaction idle/deadline timeouts.
*
* @param timerExecutorService the timer executor service
*/
@ApiStatus.Internal
@TestOnly
public void setTimerExecutorService(final @NotNull ISentryExecutorService timerExecutorService) {
if (timerExecutorService != null) {
this.timerExecutorService = timerExecutorService;
}
}

/**
* Returns the connection timeout in milliseconds.
*
Expand Down
69 changes: 29 additions & 40 deletions sentry/src/main/java/io/sentry/SentryTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.jetbrains.annotations.ApiStatus;
Expand All @@ -37,10 +36,13 @@ public final class SentryTracer implements ITransaction {
*/
private @NotNull FinishStatus finishStatus = FinishStatus.NOT_FINISHED;

private volatile @Nullable TimerTask idleTimeoutTask;
private volatile @Nullable TimerTask deadlineTimeoutTask;
private volatile @Nullable Future<?> idleTimeoutFuture;
private volatile @Nullable Future<?> deadlineTimeoutFuture;

private volatile @Nullable Timer timer = null;
// Shared executor used to schedule the timeout tasks. Null once the tracer is finished, at which
// point no more timeouts may be scheduled. It is never shut down here since it is shared
// SDK-wide.
private volatile @Nullable ISentryExecutorService timerExecutorService = null;
private final @NotNull AutoClosableReentrantLock timerLock = new AutoClosableReentrantLock();
private final @NotNull AutoClosableReentrantLock tracerLock = new AutoClosableReentrantLock();

Expand Down Expand Up @@ -99,7 +101,7 @@ public SentryTracer(

if (transactionOptions.getIdleTimeout() != null
|| transactionOptions.getDeadlineTimeout() != null) {
timer = new Timer(true);
timerExecutorService = scopes.getOptions().getTimerExecutorService();

scheduleDeadlineTimeout();
scheduleFinish();
Expand All @@ -109,22 +111,16 @@ public SentryTracer(
@Override
public void scheduleFinish() {
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
if (timer != null) {
if (timerExecutorService != null) {
final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();

if (idleTimeout != null) {
cancelIdleTimer();
isIdleFinishTimerRunning.set(true);
idleTimeoutTask =
new TimerTask() {
@Override
public void run() {
onIdleTimeoutReached();
}
};

try {
timer.schedule(idleTimeoutTask, idleTimeout);
idleTimeoutFuture =
timerExecutorService.schedule(this::onIdleTimeoutReached, idleTimeout);
} catch (Throwable e) {
scopes
.getOptions()
Expand Down Expand Up @@ -265,13 +261,12 @@ public void finish(
});
final SentryTransaction transaction = new SentryTransaction(this);

if (timer != null) {
if (timerExecutorService != null) {
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
if (timer != null) {
if (timerExecutorService != null) {
cancelIdleTimer();
cancelDeadlineTimer();
timer.cancel();
timer = null;
timerExecutorService = null;
}
}
}
Expand All @@ -295,10 +290,10 @@ public void finish(

private void cancelIdleTimer() {
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
if (idleTimeoutTask != null) {
idleTimeoutTask.cancel();
if (idleTimeoutFuture != null) {
idleTimeoutFuture.cancel(false);
isIdleFinishTimerRunning.set(false);
idleTimeoutTask = null;
idleTimeoutFuture = null;
}
}
}
Expand All @@ -307,18 +302,12 @@ private void scheduleDeadlineTimeout() {
final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();
if (deadlineTimeOut != null) {
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
if (timer != null) {
if (timerExecutorService != null) {
cancelDeadlineTimer();
isDeadlineTimerRunning.set(true);
deadlineTimeoutTask =
new TimerTask() {
@Override
public void run() {
onDeadlineTimeoutReached();
}
};
try {
timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
deadlineTimeoutFuture =
timerExecutorService.schedule(this::onDeadlineTimeoutReached, deadlineTimeOut);
} catch (Throwable e) {
scopes
.getOptions()
Expand All @@ -335,10 +324,10 @@ public void run() {

private void cancelDeadlineTimer() {
try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
if (deadlineTimeoutTask != null) {
deadlineTimeoutTask.cancel();
if (deadlineTimeoutFuture != null) {
deadlineTimeoutFuture.cancel(false);
isDeadlineTimerRunning.set(false);
deadlineTimeoutTask = null;
deadlineTimeoutFuture = null;
}
}
}
Expand Down Expand Up @@ -973,20 +962,20 @@ Span getRoot() {

@TestOnly
@Nullable
TimerTask getIdleTimeoutTask() {
return idleTimeoutTask;
Future<?> getIdleTimeoutFuture() {
return idleTimeoutFuture;
}

@TestOnly
@Nullable
TimerTask getDeadlineTimeoutTask() {
return deadlineTimeoutTask;
Future<?> getDeadlineTimeoutFuture() {
return deadlineTimeoutFuture;
}

@TestOnly
@Nullable
Timer getTimer() {
return timer;
ISentryExecutorService getTimerExecutorService() {
return timerExecutorService;
}

@TestOnly
Expand Down
16 changes: 16 additions & 0 deletions sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ class SentryExecutorServiceTest {
sentryExecutor.close(15000)
}

@Test
fun `SentryExecutorService enables removeOnCancelPolicy when requested`() {
val sentryExecutor = SentryExecutorService(null, true)
val executor = sentryExecutor.getProperty<ScheduledThreadPoolExecutor>("executorService")
assertTrue(executor.removeOnCancelPolicy)
sentryExecutor.close(15000)
}

@Test
fun `SentryExecutorService does not enable removeOnCancelPolicy by default`() {
val sentryExecutor = SentryExecutorService(null)
val executor = sentryExecutor.getProperty<ScheduledThreadPoolExecutor>("executorService")
assertFalse(executor.removeOnCancelPolicy)
sentryExecutor.close(15000)
}

@Test
fun `SentryExecutorService isClosed returns true if executor is shutdown`() {
val executor = mock<ScheduledThreadPoolExecutor>()
Expand Down
Loading
Loading