Skip to content

Commit 917418f

Browse files
43jayclaude
andcommitted
ref(profiling): Improve thread safety in PerfettoContinuousProfiler
- Lock in isRunning(), getProfilerId(), getChunkId() so all public getters are synchronized with writes in startInternal/stopInternal - Lock in reevaluateSampling() - Remove volatile from shouldSample; all accesses are now under the same lock - Replace ArrayDeque with ConcurrentLinkedDeque in PerfettoProfiler for frame measurement collections; these are written by the FrameMetrics HandlerThread and read by the executor thread in endAndCollect() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a18c69d commit 917418f

File tree

2 files changed

+34
-19
lines changed

2 files changed

+34
-19
lines changed

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,19 @@
4141
* profiling backends independent. All ProfilingManager API usage is confined to this file and {@link
4242
* PerfettoProfiler}.
4343
*
44-
* <p>Unlike the legacy profiler, this class is not used for app-start profiling. It is created
45-
* during {@code Sentry.init()}, so scopes are always available when {@link #startProfiler} is
46-
* called.
44+
* <p>Currently, this class doesn't do app-start profiling {@link SentryPerformanceProvider}.
45+
* It is created during {@code Sentry.init()}.
46+
*
47+
* <p>Thread safety: all mutable state is guarded by a single {@link
48+
* io.sentry.util.AutoClosableReentrantLock}. Public entry points ({@link #startProfiler}, {@link
49+
* #stopProfiler}, {@link #close}, {@link #onRateLimitChanged}, {@link #reevaluateSampling}, and
50+
* the getters) acquire the lock themselves and are thread-safe.
51+
* Private methods {@code startInternal} and {@code stopInternal} require the caller to hold the lock.
4752
*/
4853
@ApiStatus.Internal
4954
@RequiresApi(api = Build.VERSION_CODES.VANILLA_ICE_CREAM)
5055
public class PerfettoContinuousProfiler
5156
implements IContinuousProfiler, RateLimiter.IRateLimitObserver {
52-
5357
private static final long MAX_CHUNK_DURATION_MILLIS = 60000;
5458

5559
private final @NotNull ILogger logger;
@@ -67,7 +71,7 @@ public class PerfettoContinuousProfiler
6771
private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false);
6872
private @NotNull SentryDate startProfileChunkTimestamp =
6973
new io.sentry.SentryNanotimeDate();
70-
private volatile boolean shouldSample = true;
74+
private boolean shouldSample = true;
7175
private boolean shouldStop = false;
7276
private boolean isSampled = false;
7377
private int activeTraceCount = 0;
@@ -141,10 +145,9 @@ public void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle) {
141145
}
142146

143147
/**
144-
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead
148+
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead.
145149
*
146-
* @param rateLimiter this {@link RateLimiter} instance which you can use to check if the rate
147-
* limit is active for a specific category
150+
* @param rateLimiter the {@link RateLimiter} instance to check categories against
148151
*/
149152
@Override
150153
public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) {
@@ -173,23 +176,31 @@ public void close(final boolean isTerminating) {
173176

174177
@Override
175178
public @NotNull SentryId getProfilerId() {
176-
return profilerId;
179+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
180+
return profilerId;
181+
}
177182
}
178183

179184
@Override
180185
public @NotNull SentryId getChunkId() {
181-
return chunkId;
186+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
187+
return chunkId;
188+
}
182189
}
183190

184191
@Override
185192
public boolean isRunning() {
186-
return isRunning;
193+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
194+
return isRunning;
195+
}
187196
}
188197

189198
/**
190199
* Resolves scopes on first call. Since PerfettoContinuousProfiler is created during
191200
* Sentry.init() and never used for app-start profiling, scopes is guaranteed to be available by
192201
* the time startProfiler is called.
202+
*
203+
* <p>Caller must hold {@link #lock}.
193204
*/
194205
private @NotNull IScopes resolveScopes() {
195206
if (scopes != null && scopes != NoOpScopes.getInstance()) {
@@ -345,7 +356,9 @@ private void ensureProfiler() {
345356
}
346357

347358
public void reevaluateSampling() {
348-
shouldSample = true;
359+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
360+
shouldSample = true;
361+
}
349362
}
350363

351364
private void sendChunk(

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import io.sentry.profilemeasurements.ProfileMeasurement;
1515
import io.sentry.profilemeasurements.ProfileMeasurementValue;
1616
import java.io.File;
17-
import java.util.ArrayDeque;
1817
import java.util.HashMap;
18+
import java.util.concurrent.ConcurrentLinkedDeque;
1919
import java.util.Map;
2020
import java.util.concurrent.CountDownLatch;
2121
import java.util.concurrent.TimeUnit;
@@ -46,12 +46,14 @@ public class PerfettoProfiler {
4646
private @Nullable ProfilingResult profilingResult = null;
4747
private @Nullable CountDownLatch resultLatch = null;
4848

49-
private final @NotNull ArrayDeque<ProfileMeasurementValue> slowFrameRenderMeasurements =
50-
new ArrayDeque<>();
51-
private final @NotNull ArrayDeque<ProfileMeasurementValue> frozenFrameRenderMeasurements =
52-
new ArrayDeque<>();
53-
private final @NotNull ArrayDeque<ProfileMeasurementValue> screenFrameRateMeasurements =
54-
new ArrayDeque<>();
49+
// ConcurrentLinkedDeque because onFrameMetricCollected (HandlerThread) and endAndCollect
50+
// (executor thread) can access these concurrently.
51+
private final @NotNull ConcurrentLinkedDeque<ProfileMeasurementValue>
52+
slowFrameRenderMeasurements = new ConcurrentLinkedDeque<>();
53+
private final @NotNull ConcurrentLinkedDeque<ProfileMeasurementValue>
54+
frozenFrameRenderMeasurements = new ConcurrentLinkedDeque<>();
55+
private final @NotNull ConcurrentLinkedDeque<ProfileMeasurementValue>
56+
screenFrameRateMeasurements = new ConcurrentLinkedDeque<>();
5557
private final @NotNull Map<String, ProfileMeasurement> measurementsMap = new HashMap<>();
5658

5759
/**

0 commit comments

Comments
 (0)