Skip to content

Commit 2462226

Browse files
stefanosianostefanosiano
authored
Merge 358c43d into 8838e01 
2 parents 8838e01 + 358c43d commit 2462226

11 files changed

Lines changed: 233 additions & 37 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
### Fixes
1212

13+
- Reduce main thread work on init ([#3036](https://github.com/getsentry/sentry-java/pull/3036))
1314
- Fix SIGSEV, SIGABRT and SIGBUS crashes happening after/around the August Google Play System update, see [#2955](https://github.com/getsentry/sentry-java/issues/2955) for more details (fix provided by Native SDK bump)
1415
- Ensure DSN uses http/https protocol ([#3044](https://github.com/getsentry/sentry-java/pull/3044))
1516

@@ -36,7 +37,7 @@
3637
- Add current activity name to app context ([#2999](https://github.com/getsentry/sentry-java/pull/2999))
3738
- Add `MonitorConfig` param to `CheckInUtils.withCheckIn` ([#3038](https://github.com/getsentry/sentry-java/pull/3038))
3839
- This makes it easier to automatically create or update (upsert) monitors.
39-
40+
4041
## 6.33.1
4142

4243
### Fixes

‎sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java‎

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
public abstract class EnvelopeFileObserverIntegration implements Integration, Closeable {
1717
private @Nullable EnvelopeFileObserver observer;
1818
private @Nullable ILogger logger;
19+
private boolean isClosed = false;
20+
private final @NotNull Object startLock = new Object();
1921

2022
public static @NotNull EnvelopeFileObserverIntegration getOutboxFileObserver() {
2123
return new OutboxEnvelopeFileObserverIntegration();
@@ -37,30 +39,55 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions
3739
logger.log(
3840
SentryLevel.DEBUG, "Registering EnvelopeFileObserverIntegration for path: %s", path);
3941

40-
final OutboxSender outboxSender =
41-
new OutboxSender(
42-
hub,
43-
options.getEnvelopeReader(),
44-
options.getSerializer(),
45-
logger,
46-
options.getFlushTimeoutMillis());
47-
48-
observer =
49-
new EnvelopeFileObserver(path, outboxSender, logger, options.getFlushTimeoutMillis());
5042
try {
51-
observer.startWatching();
52-
logger.log(SentryLevel.DEBUG, "EnvelopeFileObserverIntegration installed.");
53-
} catch (Throwable e) {
54-
// it could throw eg NoSuchFileException or NullPointerException
5543
options
56-
.getLogger()
57-
.log(SentryLevel.ERROR, "Failed to initialize EnvelopeFileObserverIntegration.", e);
44+
.getExecutorService()
45+
.submit(
46+
() -> {
47+
synchronized (startLock) {
48+
if (!isClosed) {
49+
startOutboxSender(hub, options, path);
50+
}
51+
}
52+
});
53+
} catch (Throwable e) {
54+
logger.log(
55+
SentryLevel.DEBUG,
56+
"Failed to start EnvelopeFileObserverIntegration on executor thread. Starting on the calling thread.",
57+
e);
5858
}
5959
}
6060
}
6161

62+
private void startOutboxSender(
63+
final @NotNull IHub hub, final @NotNull SentryOptions options, final @NotNull String path) {
64+
final OutboxSender outboxSender =
65+
new OutboxSender(
66+
hub,
67+
options.getEnvelopeReader(),
68+
options.getSerializer(),
69+
options.getLogger(),
70+
options.getFlushTimeoutMillis());
71+
72+
observer =
73+
new EnvelopeFileObserver(
74+
path, outboxSender, options.getLogger(), options.getFlushTimeoutMillis());
75+
try {
76+
observer.startWatching();
77+
options.getLogger().log(SentryLevel.DEBUG, "EnvelopeFileObserverIntegration installed.");
78+
} catch (Throwable e) {
79+
// it could throw eg NoSuchFileException or NullPointerException
80+
options
81+
.getLogger()
82+
.log(SentryLevel.ERROR, "Failed to initialize EnvelopeFileObserverIntegration.", e);
83+
}
84+
}
85+
6286
@Override
6387
public void close() {
88+
synchronized (startLock) {
89+
isClosed = true;
90+
}
6491
if (observer != null) {
6592
observer.stopWatching();
6693

‎sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,21 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) {
4040
return;
4141
}
4242

43-
final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender =
44-
factory.create(hub, androidOptions);
45-
46-
if (sender == null) {
47-
androidOptions.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null.");
48-
return;
49-
}
50-
5143
try {
5244
Future<?> future =
5345
androidOptions
5446
.getExecutorService()
5547
.submit(
5648
() -> {
49+
final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender =
50+
factory.create(hub, androidOptions);
51+
52+
if (sender == null) {
53+
androidOptions
54+
.getLogger()
55+
.log(SentryLevel.ERROR, "SendFireAndForget factory is null.");
56+
return;
57+
}
5758
try {
5859
sender.send();
5960
} catch (Throwable e) {

‎sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,18 @@ package io.sentry.android.core
22

33
import androidx.test.ext.junit.runners.AndroidJUnit4
44
import io.sentry.Hub
5+
import io.sentry.IHub
6+
import io.sentry.ILogger
7+
import io.sentry.SentryLevel
58
import io.sentry.SentryOptions
9+
import io.sentry.test.DeferredExecutorService
10+
import io.sentry.test.ImmediateExecutorService
611
import org.junit.runner.RunWith
12+
import org.mockito.kotlin.eq
713
import org.mockito.kotlin.mock
14+
import org.mockito.kotlin.never
815
import org.mockito.kotlin.verify
16+
import org.mockito.kotlin.whenever
917
import java.io.File
1018
import java.nio.file.Files
1119
import kotlin.test.AfterTest
@@ -15,6 +23,25 @@ import kotlin.test.assertEquals
1523

1624
@RunWith(AndroidJUnit4::class)
1725
class EnvelopeFileObserverIntegrationTest {
26+
inner class Fixture {
27+
val hub: IHub = mock()
28+
private lateinit var options: SentryAndroidOptions
29+
val logger = mock<ILogger>()
30+
31+
fun getSut(optionConfiguration: (SentryAndroidOptions) -> Unit = {}): EnvelopeFileObserverIntegration {
32+
options = SentryAndroidOptions()
33+
options.setLogger(logger)
34+
options.isDebug = true
35+
optionConfiguration(options)
36+
whenever(hub.options).thenReturn(options)
37+
38+
return object : EnvelopeFileObserverIntegration() {
39+
override fun getPath(options: SentryOptions): String? = file.absolutePath
40+
}
41+
}
42+
}
43+
44+
private val fixture = Fixture()
1845

1946
private lateinit var file: File
2047

@@ -51,4 +78,44 @@ class EnvelopeFileObserverIntegrationTest {
5178
hub.close()
5279
verify(integrationMock).close()
5380
}
81+
82+
@Test
83+
fun `when hub is closed right after start, integration is not registered`() {
84+
val deferredExecutorService = DeferredExecutorService()
85+
val integration = fixture.getSut {
86+
it.executorService = deferredExecutorService
87+
}
88+
integration.register(fixture.hub, fixture.hub.options)
89+
integration.close()
90+
deferredExecutorService.runAll()
91+
verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed."))
92+
}
93+
94+
@Test
95+
fun `register with fake executor service does not install integration`() {
96+
val integration = fixture.getSut {
97+
it.executorService = mock()
98+
}
99+
integration.register(fixture.hub, fixture.hub.options)
100+
verify(fixture.logger).log(
101+
eq(SentryLevel.DEBUG),
102+
eq("Registering EnvelopeFileObserverIntegration for path: %s"),
103+
eq(file.absolutePath)
104+
)
105+
verify(fixture.logger, never()).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed."))
106+
}
107+
108+
@Test
109+
fun `register integration on the background via executor service`() {
110+
val integration = fixture.getSut {
111+
it.executorService = ImmediateExecutorService()
112+
}
113+
integration.register(fixture.hub, fixture.hub.options)
114+
verify(fixture.logger).log(
115+
eq(SentryLevel.DEBUG),
116+
eq("Registering EnvelopeFileObserverIntegration for path: %s"),
117+
eq(file.absolutePath)
118+
)
119+
verify(fixture.logger).log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed."))
120+
}
54121
}

‎sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package io.sentry.android.core
22

33
import io.sentry.IHub
44
import io.sentry.ILogger
5+
import io.sentry.ISentryExecutorService
56
import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget
67
import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory
78
import io.sentry.SentryLevel.DEBUG
9+
import io.sentry.SentryOptions
10+
import io.sentry.test.ImmediateExecutorService
811
import io.sentry.util.LazyEvaluator
912
import org.awaitility.kotlin.await
1013
import org.mockito.kotlin.any
@@ -32,11 +35,13 @@ class SendCachedEnvelopeIntegrationTest {
3235
hasStartupCrashMarker: Boolean = false,
3336
hasSender: Boolean = true,
3437
delaySend: Long = 0L,
35-
taskFails: Boolean = false
38+
taskFails: Boolean = false,
39+
mockExecutorService: ISentryExecutorService? = null
3640
): SendCachedEnvelopeIntegration {
3741
options.cacheDirPath = cacheDirPath
3842
options.setLogger(logger)
3943
options.isDebug = true
44+
options.executorService = mockExecutorService ?: SentryOptions().executorService
4045

4146
whenever(sender.send()).then {
4247
Thread.sleep(delaySend)
@@ -71,7 +76,7 @@ class SendCachedEnvelopeIntegrationTest {
7176

7277
@Test
7378
fun `when factory returns null, does nothing`() {
74-
val sut = fixture.getSut(hasSender = false)
79+
val sut = fixture.getSut(hasSender = false, mockExecutorService = ImmediateExecutorService())
7580

7681
sut.register(fixture.hub, fixture.options)
7782

@@ -81,14 +86,23 @@ class SendCachedEnvelopeIntegrationTest {
8186

8287
@Test
8388
fun `when has factory and cacheDirPath set, submits task into queue`() {
84-
val sut = fixture.getSut()
89+
val sut = fixture.getSut(mockExecutorService = ImmediateExecutorService())
8590

8691
sut.register(fixture.hub, fixture.options)
8792

8893
await.untilFalse(fixture.flag)
8994
verify(fixture.sender).send()
9095
}
9196

97+
@Test
98+
fun `when executorService is fake, does nothing`() {
99+
val sut = fixture.getSut(mockExecutorService = mock())
100+
sut.register(fixture.hub, fixture.options)
101+
102+
verify(fixture.factory, never()).create(any(), any())
103+
verify(fixture.sender, never()).send()
104+
}
105+
92106
@Test
93107
fun `when has startup crash marker, awaits the task on the calling thread`() {
94108
val sut = fixture.getSut(hasStartupCrashMarker = true)

‎sentry-test-support/api/sentry-test-support.api‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,16 @@ public final class io/sentry/SkipError : java/lang/Error {
1111
public fun <init> (Ljava/lang/String;)V
1212
}
1313

14+
public final class io/sentry/test/DeferredExecutorService : io/sentry/ISentryExecutorService {
15+
public fun <init> ()V
16+
public fun close (J)V
17+
public fun isClosed ()Z
18+
public final fun runAll ()V
19+
public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
20+
public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
21+
public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
22+
}
23+
1424
public final class io/sentry/test/ImmediateExecutorService : io/sentry/ISentryExecutorService {
1525
public fun <init> ()V
1626
public fun close (J)V

‎sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,22 @@ class ImmediateExecutorService : ISentryExecutorService {
1717
override fun close(timeoutMillis: Long) {}
1818
override fun isClosed(): Boolean = false
1919
}
20+
21+
class DeferredExecutorService : ISentryExecutorService {
22+
23+
private val runnables = ArrayList<Runnable>()
24+
25+
fun runAll() {
26+
runnables.forEach { it.run() }
27+
}
28+
29+
override fun submit(runnable: Runnable): Future<*> {
30+
runnables.add(runnable)
31+
return mock()
32+
}
33+
34+
override fun <T> submit(callable: Callable<T>): Future<T> = mock()
35+
override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = mock()
36+
override fun close(timeoutMillis: Long) {}
37+
override fun isClosed(): Boolean = false
38+
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,17 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions
6464
return;
6565
}
6666

67-
final SendFireAndForget sender = factory.create(hub, options);
68-
69-
if (sender == null) {
70-
options.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null.");
71-
return;
72-
}
73-
7467
try {
7568
options
7669
.getExecutorService()
7770
.submit(
7871
() -> {
72+
final SendFireAndForget sender = factory.create(hub, options);
73+
74+
if (sender == null) {
75+
options.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null.");
76+
return;
77+
}
7978
try {
8079
sender.send();
8180
} catch (Throwable e) {

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,18 +335,21 @@ private static boolean initConfigurations(final @NotNull SentryOptions options)
335335

336336
final File profilingTracesDir = new File(profilingTracesDirPath);
337337
profilingTracesDir.mkdirs();
338-
final File[] oldTracesDirContent = profilingTracesDir.listFiles();
338+
final long timestamp = System.currentTimeMillis();
339339

340340
try {
341341
options
342342
.getExecutorService()
343343
.submit(
344344
() -> {
345+
final File[] oldTracesDirContent = profilingTracesDir.listFiles();
345346
if (oldTracesDirContent == null) return;
346347
// Method trace files are normally deleted at the end of traces, but if that fails
347348
// for some reason we try to clear any old files here.
348349
for (File f : oldTracesDirContent) {
349-
FileUtils.deleteRecursively(f);
350+
if (f.lastModified() < timestamp) {
351+
FileUtils.deleteRecursively(f);
352+
}
350353
}
351354
});
352355
} catch (RejectedExecutionException e) {

0 commit comments

Comments
 (0)