Skip to content

Commit 38149e8

Browse files
43jayclaude
andcommitted
ref(profiling): Improve thread safety in PerfettoContinuousProfiler
- Add lock to isRunning(), getProfilerId(), getChunkId() so all public getters are synchronized with writes in startInternal/stopInternal - Add lock to reevaluateSampling() and remove volatile from shouldSample; all accesses are now under the same lock - Add Caller must hold lock javadoc to resolveScopes() - Add class-level javadoc documenting the threading/locking policy - 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 7bbe257 commit 38149e8

2 files changed

Lines changed: 32 additions & 15 deletions

File tree

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@
4545
* <p>Unlike the legacy profiler, this class is not used for app-start profiling. It is created
4646
* during {@code Sentry.init()}, so scopes are always available when {@link #startProfiler} is
4747
* called.
48+
*
49+
* <p>Thread safety: all mutable state is guarded by a single {@link
50+
* io.sentry.util.AutoClosableReentrantLock}. Public entry points ({@link #startProfiler}, {@link
51+
* #stopProfiler}, {@link #close}, {@link #onRateLimitChanged}, {@link #reevaluateSampling}, and
52+
* the getters) acquire the lock themselves. Private methods {@code startInternal} and {@code
53+
* stopInternal} require the caller to hold the lock.
4854
*/
4955
@ApiStatus.Internal
5056
@RequiresApi(api = Build.VERSION_CODES.VANILLA_ICE_CREAM)
@@ -70,7 +76,7 @@ public class PerfettoContinuousProfiler
7076
private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false);
7177
private @NotNull SentryDate startProfileChunkTimestamp =
7278
new io.sentry.SentryNanotimeDate();
73-
private volatile boolean shouldSample = true;
79+
private boolean shouldSample = true;
7480
private boolean shouldStop = false;
7581
private boolean isSampled = false;
7682
private int activeTraceCount = 0;
@@ -145,10 +151,9 @@ public void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle) {
145151
}
146152

147153
/**
148-
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead
154+
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead.
149155
*
150-
* @param rateLimiter this {@link RateLimiter} instance which you can use to check if the rate
151-
* limit is active for a specific category
156+
* @param rateLimiter the {@link RateLimiter} instance to check categories against
152157
*/
153158
@Override
154159
public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) {
@@ -177,23 +182,31 @@ public void close(final boolean isTerminating) {
177182

178183
@Override
179184
public @NotNull SentryId getProfilerId() {
180-
return profilerId;
185+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
186+
return profilerId;
187+
}
181188
}
182189

183190
@Override
184191
public @NotNull SentryId getChunkId() {
185-
return chunkId;
192+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
193+
return chunkId;
194+
}
186195
}
187196

188197
@Override
189198
public boolean isRunning() {
190-
return isRunning;
199+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
200+
return isRunning;
201+
}
191202
}
192203

193204
/**
194205
* Resolves scopes on first call. Since PerfettoContinuousProfiler is created during
195206
* Sentry.init() and never used for app-start profiling, scopes is guaranteed to be available by
196207
* the time startProfiler is called.
208+
*
209+
* <p>Caller must hold {@link #lock}.
197210
*/
198211
private @NotNull IScopes resolveScopes() {
199212
if (scopes != null && scopes != NoOpScopes.getInstance()) {
@@ -350,7 +363,9 @@ private void ensureProfiler() {
350363
}
351364

352365
public void reevaluateSampling() {
353-
shouldSample = true;
366+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
367+
shouldSample = true;
368+
}
354369
}
355370

356371
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)