Skip to content

Commit aced657

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 9595dbc commit aced657

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;
@@ -140,10 +144,9 @@ public void stopProfiler(final @NotNull ProfileLifecycle profileLifecycle) {
140144
}
141145

142146
/**
143-
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead
147+
* Stop the profiler as soon as we are rate limited, to avoid the performance overhead.
144148
*
145-
* @param rateLimiter this {@link RateLimiter} instance which you can use to check if the rate
146-
* limit is active for a specific category
149+
* @param rateLimiter the {@link RateLimiter} instance to check categories against
147150
*/
148151
@Override
149152
public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) {
@@ -172,23 +175,31 @@ public void close(final boolean isTerminating) {
172175

173176
@Override
174177
public @NotNull SentryId getProfilerId() {
175-
return profilerId;
178+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
179+
return profilerId;
180+
}
176181
}
177182

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

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

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

346357
public void reevaluateSampling() {
347-
shouldSample = true;
358+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
359+
shouldSample = true;
360+
}
348361
}
349362

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