Skip to content

Commit 42b3e9b

Browse files
stefanosianostefanosiano
authored
Merge b6c0ce1 into 8c7e8d9 
2 parents 8c7e8d9 + b6c0ce1 commit 42b3e9b

File tree

3 files changed

+16
-167
lines changed

3 files changed

+16
-167
lines changed

‎CHANGELOG.md‎

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

55
### Fixes
66

7+
- Removed SentryExecutorService limit ([#4846](https://github.com/getsentry/sentry-java/pull/4846))
78
- [ANR] Removed AndroidTransactionProfiler lock ([#4817](https://github.com/getsentry/sentry-java/pull/4817))
89
- Fix wrong .super() call in SentryTimberTree ([#4844](https://github.com/getsentry/sentry-java/pull/4844))
910

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

Lines changed: 3 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.sentry.util.AutoClosableReentrantLock;
44
import java.util.concurrent.Callable;
5-
import java.util.concurrent.CancellationException;
65
import java.util.concurrent.Future;
76
import java.util.concurrent.RejectedExecutionException;
87
import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -23,12 +22,6 @@ public final class SentryExecutorService implements ISentryExecutorService {
2322
*/
2423
private static final int INITIAL_QUEUE_SIZE = 40;
2524

26-
/**
27-
* By default, the work queue is unbounded so it can grow as much as the memory allows. We want to
28-
* limit it by 271 which would be x8 times growth from the default initial capacity.
29-
*/
30-
private static final int MAX_QUEUE_SIZE = 271;
31-
3225
private final @NotNull ScheduledThreadPoolExecutor executorService;
3326
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
3427

@@ -56,43 +49,19 @@ public SentryExecutorService() {
5649
@Override
5750
public @NotNull Future<?> submit(final @NotNull Runnable runnable)
5851
throws RejectedExecutionException {
59-
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
60-
return executorService.submit(runnable);
61-
}
62-
if (options != null) {
63-
options
64-
.getLogger()
65-
.log(SentryLevel.WARNING, "Task " + runnable + " rejected from " + executorService);
66-
}
67-
return new CancelledFuture<>();
52+
return executorService.submit(runnable);
6853
}
6954

7055
@Override
7156
public @NotNull <T> Future<T> submit(final @NotNull Callable<T> callable)
7257
throws RejectedExecutionException {
73-
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
74-
return executorService.submit(callable);
75-
}
76-
if (options != null) {
77-
options
78-
.getLogger()
79-
.log(SentryLevel.WARNING, "Task " + callable + " rejected from " + executorService);
80-
}
81-
return new CancelledFuture<>();
58+
return executorService.submit(callable);
8259
}
8360

8461
@Override
8562
public @NotNull Future<?> schedule(final @NotNull Runnable runnable, final long delayMillis)
8663
throws RejectedExecutionException {
87-
if (executorService.getQueue().size() < MAX_QUEUE_SIZE) {
88-
return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS);
89-
}
90-
if (options != null) {
91-
options
92-
.getLogger()
93-
.log(SentryLevel.WARNING, "Task " + runnable + " rejected from " + executorService);
94-
}
95-
return new CancelledFuture<>();
64+
return executorService.schedule(runnable, delayMillis, TimeUnit.MILLISECONDS);
9665
}
9766

9867
@Override
@@ -158,31 +127,4 @@ private static final class SentryExecutorServiceThreadFactory implements ThreadF
158127
return ret;
159128
}
160129
}
161-
162-
private static final class CancelledFuture<T> implements Future<T> {
163-
@Override
164-
public boolean cancel(final boolean mayInterruptIfRunning) {
165-
return true;
166-
}
167-
168-
@Override
169-
public boolean isCancelled() {
170-
return true;
171-
}
172-
173-
@Override
174-
public boolean isDone() {
175-
return true;
176-
}
177-
178-
@Override
179-
public T get() {
180-
throw new CancellationException();
181-
}
182-
183-
@Override
184-
public T get(final long timeout, final @NotNull TimeUnit unit) {
185-
throw new CancellationException();
186-
}
187-
}
188130
}

‎sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt‎

Lines changed: 12 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package io.sentry
22

33
import io.sentry.test.getProperty
4-
import java.util.concurrent.BlockingQueue
5-
import java.util.concurrent.Callable
6-
import java.util.concurrent.CancellationException
74
import java.util.concurrent.LinkedBlockingQueue
85
import java.util.concurrent.ScheduledThreadPoolExecutor
6+
import java.util.concurrent.TimeUnit
97
import java.util.concurrent.atomic.AtomicBoolean
108
import kotlin.test.Test
119
import kotlin.test.assertEquals
12-
import kotlin.test.assertFailsWith
1310
import kotlin.test.assertFalse
1411
import kotlin.test.assertTrue
1512
import org.awaitility.kotlin.await
@@ -109,108 +106,6 @@ class SentryExecutorServiceTest {
109106
assertFalse(sentryExecutor.isClosed)
110107
}
111108

112-
@Test
113-
fun `SentryExecutorService submit runnable returns cancelled future when queue size exceeds limit`() {
114-
val queue = mock<BlockingQueue<Runnable>>()
115-
whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271)
116-
117-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
118-
119-
val options = mock<SentryOptions>()
120-
val logger = mock<ILogger>()
121-
whenever(options.logger).thenReturn(logger)
122-
123-
val sentryExecutor = SentryExecutorService(executor, options)
124-
val future = sentryExecutor.submit {}
125-
126-
assertTrue(future.isCancelled)
127-
assertTrue(future.isDone)
128-
assertFailsWith<CancellationException> { future.get() }
129-
verify(executor, never()).submit(any<Runnable>())
130-
verify(logger).log(any<SentryLevel>(), any<String>())
131-
}
132-
133-
@Test
134-
fun `SentryExecutorService submit runnable accepts when queue size is within limit`() {
135-
val queue = mock<BlockingQueue<Runnable>>()
136-
whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271)
137-
138-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
139-
140-
val sentryExecutor = SentryExecutorService(executor, null)
141-
sentryExecutor.submit {}
142-
143-
verify(executor).submit(any<Runnable>())
144-
}
145-
146-
@Test
147-
fun `SentryExecutorService submit callable returns cancelled future when queue size exceeds limit`() {
148-
val queue = mock<BlockingQueue<Runnable>>()
149-
whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271)
150-
151-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
152-
153-
val options = mock<SentryOptions>()
154-
val logger = mock<ILogger>()
155-
whenever(options.logger).thenReturn(logger)
156-
157-
val sentryExecutor = SentryExecutorService(executor, options)
158-
val future = sentryExecutor.submit(Callable { "result" })
159-
160-
assertTrue(future.isCancelled)
161-
assertTrue(future.isDone)
162-
assertFailsWith<CancellationException> { future.get() }
163-
verify(executor, never()).submit(any<Callable<String>>())
164-
verify(logger).log(any<SentryLevel>(), any<String>())
165-
}
166-
167-
@Test
168-
fun `SentryExecutorService submit callable accepts when queue size is within limit`() {
169-
val queue = mock<BlockingQueue<Runnable>>()
170-
whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271)
171-
172-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
173-
174-
val sentryExecutor = SentryExecutorService(executor, null)
175-
sentryExecutor.submit(Callable { "result" })
176-
177-
verify(executor).submit(any<Callable<String>>())
178-
}
179-
180-
@Test
181-
fun `SentryExecutorService schedule returns cancelled future when queue size exceeds limit`() {
182-
val queue = mock<BlockingQueue<Runnable>>()
183-
whenever(queue.size).thenReturn(272) // Above MAX_QUEUE_SIZE (271)
184-
185-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
186-
187-
val options = mock<SentryOptions>()
188-
val logger = mock<ILogger>()
189-
whenever(options.logger).thenReturn(logger)
190-
191-
val sentryExecutor = SentryExecutorService(executor, options)
192-
val future = sentryExecutor.schedule({}, 1000L)
193-
194-
assertTrue(future.isCancelled)
195-
assertTrue(future.isDone)
196-
assertFailsWith<CancellationException> { future.get() }
197-
verify(executor, never()).schedule(any<Runnable>(), any(), any())
198-
verify(logger).log(any<SentryLevel>(), any<String>())
199-
}
200-
201-
@Test
202-
fun `SentryExecutorService schedule accepts when queue size is within limit`() {
203-
val queue = mock<BlockingQueue<Runnable>>()
204-
whenever(queue.size).thenReturn(270) // Below MAX_QUEUE_SIZE (271)
205-
206-
val executor = mock<ScheduledThreadPoolExecutor> { on { getQueue() } doReturn queue }
207-
208-
val sentryExecutor = SentryExecutorService(executor, null)
209-
sentryExecutor.schedule({}, 1000L)
210-
211-
verify(executor).schedule(any<Runnable>(), any(), any())
212-
}
213-
214109
@Test
215110
fun `SentryExecutorService prewarm schedules dummy tasks and clears queue`() {
216111
val executor = ScheduledThreadPoolExecutor(1)
@@ -225,4 +120,15 @@ class SentryExecutorServiceTest {
225120
// the queue should be empty
226121
assertEquals(0, executor.queue.size)
227122
}
123+
124+
@Test
125+
fun `SentryExecutorService runs any number of job`() {
126+
val sentryExecutor = SentryExecutorService()
127+
var called = false
128+
// Post 1k jobs after 1 day, to test new jobs are accepted
129+
repeat(1000) { sentryExecutor.schedule({}, TimeUnit.DAYS.toMillis(1)) }
130+
sentryExecutor.submit { called = true }
131+
await.until { called }
132+
assertTrue(called)
133+
}
228134
}

0 commit comments

Comments
 (0)