Skip to content

Commit 818a27d

Browse files
lciangetsentry-bot
andauthored
Handle RejectedExecutionException everywhere (#4747)
Co-authored-by: Sentry Github Bot <[email protected]>
1 parent e4db821 commit 818a27d

File tree

7 files changed

+123
-67
lines changed

7 files changed

+123
-67
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Improvements
66

7+
- Handle `RejectedExecutionException` everywhere ([#4747](https://github.com/getsentry/sentry-java/pull/4747))
78
- Mark `SentryEnvelope` as not internal ([#4748](https://github.com/getsentry/sentry-java/pull/4748))
89

910
## 8.22.0

sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java

Lines changed: 53 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,7 @@
44
import android.content.pm.PackageInfo;
55
import android.content.pm.PackageManager;
66
import android.os.Build;
7-
import io.sentry.DateUtils;
8-
import io.sentry.EventProcessor;
9-
import io.sentry.Hint;
10-
import io.sentry.IpAddressUtils;
11-
import io.sentry.NoOpLogger;
12-
import io.sentry.SentryAttributeType;
13-
import io.sentry.SentryBaseEvent;
14-
import io.sentry.SentryEvent;
15-
import io.sentry.SentryLevel;
16-
import io.sentry.SentryLogEvent;
17-
import io.sentry.SentryLogEventAttributeValue;
18-
import io.sentry.SentryReplayEvent;
7+
import io.sentry.*;
198
import io.sentry.android.core.internal.util.AndroidThreadChecker;
209
import io.sentry.android.core.performance.AppStartMetrics;
2110
import io.sentry.android.core.performance.TimeSpan;
@@ -37,6 +26,7 @@
3726
import java.util.concurrent.ExecutorService;
3827
import java.util.concurrent.Executors;
3928
import java.util.concurrent.Future;
29+
import java.util.concurrent.RejectedExecutionException;
4030
import org.jetbrains.annotations.NotNull;
4131
import org.jetbrains.annotations.Nullable;
4232
import org.jetbrains.annotations.TestOnly;
@@ -47,7 +37,7 @@ final class DefaultAndroidEventProcessor implements EventProcessor {
4737

4838
private final @NotNull BuildInfoProvider buildInfoProvider;
4939
private final @NotNull SentryAndroidOptions options;
50-
private final @NotNull Future<DeviceInfoUtil> deviceInfoUtil;
40+
private final @Nullable Future<DeviceInfoUtil> deviceInfoUtil;
5141
private final @NotNull LazyEvaluator<String> deviceFamily =
5242
new LazyEvaluator<>(() -> ContextUtils.getFamily(NoOpLogger.getInstance()));
5343

@@ -65,9 +55,16 @@ public DefaultAndroidEventProcessor(
6555
// don't ref. to method reference, theres a bug on it
6656
// noinspection Convert2MethodRef
6757
// some device info performs disk I/O, but it's result is cached, let's pre-cache it
58+
@Nullable Future<DeviceInfoUtil> deviceInfoUtil;
6859
final @NotNull ExecutorService executorService = Executors.newSingleThreadExecutor();
69-
this.deviceInfoUtil =
70-
executorService.submit(() -> DeviceInfoUtil.getInstance(this.context, options));
60+
try {
61+
deviceInfoUtil =
62+
executorService.submit(() -> DeviceInfoUtil.getInstance(this.context, options));
63+
} catch (RejectedExecutionException e) {
64+
deviceInfoUtil = null;
65+
options.getLogger().log(SentryLevel.WARNING, "Device info caching task rejected.", e);
66+
}
67+
this.deviceInfoUtil = deviceInfoUtil;
7168
executorService.shutdown();
7269
}
7370

@@ -181,25 +178,34 @@ private void setDevice(
181178
final boolean errorEvent,
182179
final boolean applyScopeData) {
183180
if (event.getContexts().getDevice() == null) {
184-
try {
185-
event
186-
.getContexts()
187-
.setDevice(deviceInfoUtil.get().collectDeviceInformation(errorEvent, applyScopeData));
188-
} catch (Throwable e) {
189-
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info", e);
181+
if (deviceInfoUtil != null) {
182+
try {
183+
event
184+
.getContexts()
185+
.setDevice(deviceInfoUtil.get().collectDeviceInformation(errorEvent, applyScopeData));
186+
} catch (Throwable e) {
187+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info", e);
188+
}
189+
} else {
190+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info");
190191
}
191192
mergeOS(event);
192193
}
193194
}
194195

195196
private void mergeOS(final @NotNull SentryBaseEvent event) {
196197
final OperatingSystem currentOS = event.getContexts().getOperatingSystem();
197-
try {
198-
final OperatingSystem androidOS = deviceInfoUtil.get().getOperatingSystem();
199-
// make Android OS the main OS using the 'os' key
200-
event.getContexts().setOperatingSystem(androidOS);
201-
} catch (Throwable e) {
202-
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve os system", e);
198+
199+
if (deviceInfoUtil != null) {
200+
try {
201+
final OperatingSystem androidOS = deviceInfoUtil.get().getOperatingSystem();
202+
// make Android OS the main OS using the 'os' key
203+
event.getContexts().setOperatingSystem(androidOS);
204+
} catch (Throwable e) {
205+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve os system", e);
206+
}
207+
} else {
208+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info");
203209
}
204210

205211
if (currentOS != null) {
@@ -284,10 +290,14 @@ private void setPackageInfo(final @NotNull SentryBaseEvent event, final @NotNull
284290
setDist(event, versionCode);
285291

286292
@Nullable DeviceInfoUtil deviceInfoUtil = null;
287-
try {
288-
deviceInfoUtil = this.deviceInfoUtil.get();
289-
} catch (Throwable e) {
290-
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info", e);
293+
if (this.deviceInfoUtil != null) {
294+
try {
295+
deviceInfoUtil = this.deviceInfoUtil.get();
296+
} catch (Throwable e) {
297+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info", e);
298+
}
299+
} else {
300+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info");
291301
}
292302

293303
ContextUtils.setAppPackageInfo(packageInfo, buildInfoProvider, deviceInfoUtil, app);
@@ -331,16 +341,20 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) {
331341
}
332342

333343
private void setSideLoadedInfo(final @NotNull SentryBaseEvent event) {
334-
try {
335-
final ContextUtils.SideLoadedInfo sideLoadedInfo = deviceInfoUtil.get().getSideLoadedInfo();
336-
if (sideLoadedInfo != null) {
337-
final @NotNull Map<String, String> tags = sideLoadedInfo.asTags();
338-
for (Map.Entry<String, String> entry : tags.entrySet()) {
339-
event.setTag(entry.getKey(), entry.getValue());
344+
if (deviceInfoUtil != null) {
345+
try {
346+
final ContextUtils.SideLoadedInfo sideLoadedInfo = deviceInfoUtil.get().getSideLoadedInfo();
347+
if (sideLoadedInfo != null) {
348+
final @NotNull Map<String, String> tags = sideLoadedInfo.asTags();
349+
for (Map.Entry<String, String> entry : tags.entrySet()) {
350+
event.setTag(entry.getKey(), entry.getValue());
351+
}
340352
}
353+
} catch (Throwable e) {
354+
options.getLogger().log(SentryLevel.ERROR, "Error getting side loaded info.", e);
341355
}
342-
} catch (Throwable e) {
343-
options.getLogger().log(SentryLevel.ERROR, "Error getting side loaded info.", e);
356+
} else {
357+
options.getLogger().log(SentryLevel.ERROR, "Failed to retrieve device info");
344358
}
345359
}
346360

sentry/src/main/java/io/sentry/Scopes.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,15 @@
55
import io.sentry.hints.SessionStartHint;
66
import io.sentry.logger.ILoggerApi;
77
import io.sentry.logger.LoggerApi;
8-
import io.sentry.protocol.Feedback;
9-
import io.sentry.protocol.SentryId;
10-
import io.sentry.protocol.SentryTransaction;
11-
import io.sentry.protocol.User;
8+
import io.sentry.protocol.*;
129
import io.sentry.transport.RateLimiter;
1310
import io.sentry.util.HintUtils;
1411
import io.sentry.util.Objects;
1512
import io.sentry.util.SpanUtils;
1613
import io.sentry.util.TracingUtils;
1714
import java.io.Closeable;
1815
import java.util.List;
16+
import java.util.concurrent.RejectedExecutionException;
1917
import org.jetbrains.annotations.ApiStatus;
2018
import org.jetbrains.annotations.NotNull;
2119
import org.jetbrains.annotations.Nullable;
@@ -449,8 +447,18 @@ public void close(final boolean isRestarting) {
449447
getOptions().getConnectionStatusProvider().close();
450448
final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService();
451449
if (isRestarting) {
452-
executorService.submit(
453-
() -> executorService.close(getOptions().getShutdownTimeoutMillis()));
450+
try {
451+
executorService.submit(
452+
() -> executorService.close(getOptions().getShutdownTimeoutMillis()));
453+
} catch (RejectedExecutionException e) {
454+
getOptions()
455+
.getLogger()
456+
.log(
457+
SentryLevel.WARNING,
458+
"Failed to submit executor service shutdown task during restart. Shutting down synchronously.",
459+
e);
460+
executorService.close(getOptions().getShutdownTimeoutMillis());
461+
}
454462
} else {
455463
executorService.close(getOptions().getShutdownTimeoutMillis());
456464
}

sentry/src/main/java/io/sentry/SentryExecutorService.java

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ public SentryExecutorService() {
5454
}
5555

5656
@Override
57-
public @NotNull Future<?> submit(final @NotNull Runnable runnable) {
57+
public @NotNull Future<?> submit(final @NotNull Runnable runnable)
58+
throws RejectedExecutionException {
5859
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
5960
return executorService.submit(runnable);
6061
}
61-
// TODO: maybe RejectedExecutionException?
6262
if (options != null) {
6363
options
6464
.getLogger()
@@ -68,11 +68,11 @@ public SentryExecutorService() {
6868
}
6969

7070
@Override
71-
public @NotNull <T> Future<T> submit(final @NotNull Callable<T> callable) {
71+
public @NotNull <T> Future<T> submit(final @NotNull Callable<T> callable)
72+
throws RejectedExecutionException {
7273
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
7374
return executorService.submit(callable);
7475
}
75-
// TODO: maybe RejectedExecutionException?
7676
if (options != null) {
7777
options
7878
.getLogger()
@@ -82,11 +82,11 @@ public SentryExecutorService() {
8282
}
8383

8484
@Override
85-
public @NotNull Future<?> schedule(final @NotNull Runnable runnable, final long delayMillis) {
85+
public @NotNull Future<?> schedule(final @NotNull Runnable runnable, final long delayMillis)
86+
throws RejectedExecutionException {
8687
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
8788
return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS);
8889
}
89-
// TODO: maybe RejectedExecutionException?
9090
if (options != null) {
9191
options
9292
.getLogger()
@@ -122,20 +122,30 @@ public boolean isClosed() {
122122
@SuppressWarnings({"FutureReturnValueIgnored"})
123123
@Override
124124
public void prewarm() {
125-
executorService.submit(
126-
() -> {
127-
try {
128-
// schedule a bunch of dummy runnables in the future that will never execute to trigger
129-
// queue growth and then purge the queue
130-
for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) {
131-
final Future<?> future = executorService.schedule(dummyRunnable, 365L, TimeUnit.DAYS);
132-
future.cancel(true);
125+
try {
126+
executorService.submit(
127+
() -> {
128+
try {
129+
// schedule a bunch of dummy runnables in the future that will never execute to
130+
// trigger
131+
// queue growth and then purge the queue
132+
for (int i = 0; i < INITIAL_QUEUE_SIZE; i++) {
133+
final Future<?> future =
134+
executorService.schedule(dummyRunnable, 365L, TimeUnit.DAYS);
135+
future.cancel(true);
136+
}
137+
executorService.purge();
138+
} catch (RejectedExecutionException ignored) {
139+
// ignore
133140
}
134-
executorService.purge();
135-
} catch (RejectedExecutionException ignored) {
136-
// ignore
137-
}
138-
});
141+
});
142+
} catch (RejectedExecutionException e) {
143+
if (options != null) {
144+
options
145+
.getLogger()
146+
.log(SentryLevel.WARNING, "Prewarm task rejected from " + executorService, e);
147+
}
148+
}
139149
}
140150

141151
private static final class SentryExecutorServiceThreadFactory implements ThreadFactory {

sentry/src/main/java/io/sentry/backpressure/BackpressureMonitor.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import io.sentry.SentryOptions;
88
import io.sentry.util.AutoClosableReentrantLock;
99
import java.util.concurrent.Future;
10+
import java.util.concurrent.RejectedExecutionException;
1011
import org.jetbrains.annotations.NotNull;
1112
import org.jetbrains.annotations.Nullable;
1213

@@ -79,7 +80,13 @@ private void reschedule(final int delay) {
7980
final @NotNull ISentryExecutorService executorService = sentryOptions.getExecutorService();
8081
if (!executorService.isClosed()) {
8182
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
82-
latestScheduledRun = executorService.schedule(this, delay);
83+
try {
84+
latestScheduledRun = executorService.schedule(this, delay);
85+
} catch (RejectedExecutionException e) {
86+
sentryOptions
87+
.getLogger()
88+
.log(SentryLevel.WARNING, "Backpressure monitor reschedule task rejected", e);
89+
}
8390
}
8491
}
8592
}

sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.Queue;
1616
import java.util.concurrent.ConcurrentLinkedQueue;
1717
import java.util.concurrent.Future;
18+
import java.util.concurrent.RejectedExecutionException;
1819
import java.util.concurrent.TimeUnit;
1920
import org.jetbrains.annotations.NotNull;
2021
import org.jetbrains.annotations.Nullable;
@@ -76,7 +77,14 @@ private void maybeSchedule(boolean forceSchedule, boolean immediately) {
7677
|| latestScheduledFlush.isCancelled()) {
7778
hasScheduled = true;
7879
final int flushAfterMs = immediately ? 0 : FLUSH_AFTER_MS;
79-
scheduledFlush = executorService.schedule(new BatchRunnable(), flushAfterMs);
80+
try {
81+
scheduledFlush = executorService.schedule(new BatchRunnable(), flushAfterMs);
82+
} catch (RejectedExecutionException e) {
83+
hasScheduled = false;
84+
options
85+
.getLogger()
86+
.log(SentryLevel.WARNING, "Logs batch processor flush task rejected", e);
87+
}
8088
}
8189
}
8290
}

sentry/src/main/java/io/sentry/transport/QueuedThreadPoolExecutor.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.concurrent.CancellationException;
99
import java.util.concurrent.Future;
1010
import java.util.concurrent.LinkedBlockingQueue;
11+
import java.util.concurrent.RejectedExecutionException;
1112
import java.util.concurrent.RejectedExecutionHandler;
1213
import java.util.concurrent.ThreadFactory;
1314
import java.util.concurrent.ThreadPoolExecutor;
@@ -65,7 +66,14 @@ public QueuedThreadPoolExecutor(
6566
public Future<?> submit(final @NotNull Runnable task) {
6667
if (isSchedulingAllowed()) {
6768
unfinishedTasksCount.increment();
68-
return super.submit(task);
69+
try {
70+
return super.submit(task);
71+
} catch (RejectedExecutionException e) {
72+
unfinishedTasksCount.decrement();
73+
lastRejectTimestamp = dateProvider.now();
74+
logger.log(SentryLevel.WARNING, "Submit rejected by thread pool executor", e);
75+
return new CancelledFuture<>();
76+
}
6977
} else {
7078
lastRejectTimestamp = dateProvider.now();
7179
// if the thread pool is full, we don't cache it

0 commit comments

Comments
 (0)