Skip to content

Commit

Permalink
Merge eff87ba into 61a31d5
Browse files Browse the repository at this point in the history
  • Loading branch information
@adinauer
adinauer authored Sep 26, 2024
2 parents 61a31d5 + eff87ba commit 4a331fa
Show file tree
Hide file tree
Showing 63 changed files with 1,049 additions and 760 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- This will reduce the number of spans created by the SDK
- `options.experimental.sessionReplay.errorSampleRate` was renamed to `options.experimental.sessionReplay.onErrorSampleRate` ([#3637](https://github.com/getsentry/sentry-java/pull/3637))
- Manifest option `io.sentry.session-replay.error-sample-rate` was renamed to `io.sentry.session-replay.on-error-sample-rate` ([#3637](https://github.com/getsentry/sentry-java/pull/3637))
- Replace `synchronized` methods and blocks with `ReentrantLock` (`AutoClosableReentrantLock`) ([#3715](https://github.com/getsentry/sentry-java/pull/3715))
- If you are subclassing any Sentry classes, please check if the parent class used `synchronized` before. Please make sure to use the same lock object as the parent class in that case.

### Features

Expand Down
4 changes: 4 additions & 0 deletions sentry-android-core/api/sentry-android-core.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class io/sentry/android/core/AndroidMemoryCollector : io/sentry/IPerforma
}

public class io/sentry/android/core/AndroidProfiler {
protected final field lock Lio/sentry/util/AutoClosableReentrantLock;
public fun <init> (Ljava/lang/String;ILio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ISentryExecutorService;Lio/sentry/ILogger;Lio/sentry/android/core/BuildInfoProvider;)V
public fun close ()V
public fun endAndCollect (ZLjava/util/List;)Lio/sentry/android/core/AndroidProfiler$ProfileEndData;
Expand Down Expand Up @@ -194,6 +195,7 @@ public final class io/sentry/android/core/DeviceInfoUtil {
}

public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : io/sentry/Integration, java/io/Closeable {
protected final field startLock Lio/sentry/util/AutoClosableReentrantLock;
public fun <init> ()V
public fun close ()V
public static fun getOutboxFileObserver ()Lio/sentry/android/core/EnvelopeFileObserverIntegration;
Expand Down Expand Up @@ -355,6 +357,7 @@ public final class io/sentry/android/core/SentryPerformanceProvider {
}

public class io/sentry/android/core/SpanFrameMetricsCollector : io/sentry/IPerformanceContinuousCollector, io/sentry/android/core/internal/util/SentryFrameMetricsCollector$FrameMetricsCollectorListener {
protected final field lock Lio/sentry/util/AutoClosableReentrantLock;
public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;)V
public fun clear ()V
public fun onFrameMetricCollected (JJJJZZF)V
Expand Down Expand Up @@ -431,6 +434,7 @@ public class io/sentry/android/core/performance/ActivityLifecycleTimeSpan : java
}

public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/android/core/performance/ActivityLifecycleCallbacksAdapter {
public static final field staticLock Lio/sentry/util/AutoClosableReentrantLock;
public fun <init> ()V
public fun addActivityLifecycleTimeSpans (Lio/sentry/android/core/performance/ActivityLifecycleTimeSpan;)V
public fun clear ()V
Expand Down
46 changes: 32 additions & 14 deletions sentry-android-core/src/main/java/io/sentry/android/core/ActivityBreadcrumbsIntegration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
import io.sentry.Breadcrumb;
import io.sentry.Hint;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.Integration;
import io.sentry.SentryLevel;
import io.sentry.SentryOptions;
import io.sentry.util.AutoClosableReentrantLock;
import io.sentry.util.Objects;
import java.io.Closeable;
import java.io.IOException;
Expand All @@ -25,7 +27,9 @@ public final class ActivityBreadcrumbsIntegration
private final @NotNull Application application;
private @Nullable IScopes scopes;
private boolean enabled;
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();

// TODO check if locking is even required at all for lifecycle methods
public ActivityBreadcrumbsIntegration(final @NotNull Application application) {
this.application = Objects.requireNonNull(application, "Application is required");
}
Expand Down Expand Up @@ -64,40 +68,54 @@ public void close() throws IOException {
}

@Override
public synchronized void onActivityCreated(
public void onActivityCreated(
final @NotNull Activity activity, final @Nullable Bundle savedInstanceState) {
addBreadcrumb(activity, "created");
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "created");
}
}

@Override
public synchronized void onActivityStarted(final @NotNull Activity activity) {
addBreadcrumb(activity, "started");
public void onActivityStarted(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "started");
}
}

@Override
public synchronized void onActivityResumed(final @NotNull Activity activity) {
addBreadcrumb(activity, "resumed");
public void onActivityResumed(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "resumed");
}
}

@Override
public synchronized void onActivityPaused(final @NotNull Activity activity) {
addBreadcrumb(activity, "paused");
public void onActivityPaused(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "paused");
}
}

@Override
public synchronized void onActivityStopped(final @NotNull Activity activity) {
addBreadcrumb(activity, "stopped");
public void onActivityStopped(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "stopped");
}
}

@Override
public synchronized void onActivitySaveInstanceState(
public void onActivitySaveInstanceState(
final @NotNull Activity activity, final @NotNull Bundle outState) {
addBreadcrumb(activity, "saveInstanceState");
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "saveInstanceState");
}
}

@Override
public synchronized void onActivityDestroyed(final @NotNull Activity activity) {
addBreadcrumb(activity, "destroyed");
public void onActivityDestroyed(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
addBreadcrumb(activity, "destroyed");
}
}

private void addBreadcrumb(final @NotNull Activity activity, final @NotNull String state) {
Expand Down
123 changes: 66 additions & 57 deletions sentry-android-core/src/main/java/io/sentry/android/core/ActivityFramesTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
import android.app.Activity;
import android.util.SparseIntArray;
import androidx.core.app.FrameMetricsAggregator;
import io.sentry.ISentryLifecycleToken;
import io.sentry.MeasurementUnit;
import io.sentry.SentryLevel;
import io.sentry.android.core.internal.util.AndroidThreadChecker;
import io.sentry.protocol.MeasurementValue;
import io.sentry.protocol.SentryId;
import io.sentry.util.AutoClosableReentrantLock;
import java.util.HashMap;
import java.util.Map;
import java.util.WeakHashMap;
Expand Down Expand Up @@ -37,6 +39,7 @@ public final class ActivityFramesTracker {
new WeakHashMap<>();

private final @NotNull MainLooperHandler handler;
protected @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();

public ActivityFramesTracker(
final @NotNull io.sentry.util.LoadClass loadClass,
Expand Down Expand Up @@ -78,13 +81,15 @@ public boolean isFrameMetricsAggregatorAvailable() {
}

@SuppressWarnings("NullAway")
public synchronized void addActivity(final @NotNull Activity activity) {
if (!isFrameMetricsAggregatorAvailable()) {
return;
}
public void addActivity(final @NotNull Activity activity) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
if (!isFrameMetricsAggregatorAvailable()) {
return;
}

runSafelyOnUiThread(() -> frameMetricsAggregator.add(activity), "FrameMetricsAggregator.add");
snapshotFrameCountsAtStart(activity);
runSafelyOnUiThread(() -> frameMetricsAggregator.add(activity), "FrameMetricsAggregator.add");
snapshotFrameCountsAtStart(activity);
}
}

private void snapshotFrameCountsAtStart(final @NotNull Activity activity) {
Expand Down Expand Up @@ -132,45 +137,46 @@ private void snapshotFrameCountsAtStart(final @NotNull Activity activity) {
}

@SuppressWarnings("NullAway")
public synchronized void setMetrics(
final @NotNull Activity activity, final @NotNull SentryId transactionId) {
if (!isFrameMetricsAggregatorAvailable()) {
return;
}
public void setMetrics(final @NotNull Activity activity, final @NotNull SentryId transactionId) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
if (!isFrameMetricsAggregatorAvailable()) {
return;
}

// NOTE: removing an activity does not reset the frame counts, only reset() does
// throws IllegalArgumentException when attempting to remove
// OnFrameMetricsAvailableListener
// that was never added.
// there's no contains method.
// throws NullPointerException when attempting to remove
// OnFrameMetricsAvailableListener and
// there was no
// Observers, See
// https://android.googlesource.com/platform/frameworks/base/+/140ff5ea8e2d99edc3fbe63a43239e459334c76b
runSafelyOnUiThread(() -> frameMetricsAggregator.remove(activity), null);

final @Nullable FrameCounts frameCounts = diffFrameCountsAtEnd(activity);

if (frameCounts == null
|| (frameCounts.totalFrames == 0
&& frameCounts.slowFrames == 0
&& frameCounts.frozenFrames == 0)) {
return;
}
// NOTE: removing an activity does not reset the frame counts, only reset() does
// throws IllegalArgumentException when attempting to remove
// OnFrameMetricsAvailableListener
// that was never added.
// there's no contains method.
// throws NullPointerException when attempting to remove
// OnFrameMetricsAvailableListener and
// there was no
// Observers, See
// https://android.googlesource.com/platform/frameworks/base/+/140ff5ea8e2d99edc3fbe63a43239e459334c76b
runSafelyOnUiThread(() -> frameMetricsAggregator.remove(activity), null);

final @Nullable FrameCounts frameCounts = diffFrameCountsAtEnd(activity);

if (frameCounts == null
|| (frameCounts.totalFrames == 0
&& frameCounts.slowFrames == 0
&& frameCounts.frozenFrames == 0)) {
return;
}

final MeasurementValue tfValues =
new MeasurementValue(frameCounts.totalFrames, MeasurementUnit.NONE);
final MeasurementValue sfValues =
new MeasurementValue(frameCounts.slowFrames, MeasurementUnit.NONE);
final MeasurementValue ffValues =
new MeasurementValue(frameCounts.frozenFrames, MeasurementUnit.NONE);
final Map<String, @NotNull MeasurementValue> measurements = new HashMap<>();
measurements.put(MeasurementValue.KEY_FRAMES_TOTAL, tfValues);
measurements.put(MeasurementValue.KEY_FRAMES_SLOW, sfValues);
measurements.put(MeasurementValue.KEY_FRAMES_FROZEN, ffValues);

activityMeasurements.put(transactionId, measurements);
final MeasurementValue tfValues =
new MeasurementValue(frameCounts.totalFrames, MeasurementUnit.NONE);
final MeasurementValue sfValues =
new MeasurementValue(frameCounts.slowFrames, MeasurementUnit.NONE);
final MeasurementValue ffValues =
new MeasurementValue(frameCounts.frozenFrames, MeasurementUnit.NONE);
final Map<String, @NotNull MeasurementValue> measurements = new HashMap<>();
measurements.put(MeasurementValue.KEY_FRAMES_TOTAL, tfValues);
measurements.put(MeasurementValue.KEY_FRAMES_SLOW, sfValues);
measurements.put(MeasurementValue.KEY_FRAMES_FROZEN, ffValues);

activityMeasurements.put(transactionId, measurements);
}
}

private @Nullable FrameCounts diffFrameCountsAtEnd(final @NotNull Activity activity) {
Expand All @@ -192,25 +198,28 @@ public synchronized void setMetrics(
}

@Nullable
public synchronized Map<String, @NotNull MeasurementValue> takeMetrics(
final @NotNull SentryId transactionId) {
if (!isFrameMetricsAggregatorAvailable()) {
return null;
}
public Map<String, @NotNull MeasurementValue> takeMetrics(final @NotNull SentryId transactionId) {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
if (!isFrameMetricsAggregatorAvailable()) {
return null;
}

final Map<String, @NotNull MeasurementValue> stringMeasurementValueMap =
activityMeasurements.get(transactionId);
activityMeasurements.remove(transactionId);
return stringMeasurementValueMap;
final Map<String, @NotNull MeasurementValue> stringMeasurementValueMap =
activityMeasurements.get(transactionId);
activityMeasurements.remove(transactionId);
return stringMeasurementValueMap;
}
}

@SuppressWarnings("NullAway")
public synchronized void stop() {
if (isFrameMetricsAggregatorAvailable()) {
runSafelyOnUiThread(() -> frameMetricsAggregator.stop(), "FrameMetricsAggregator.stop");
frameMetricsAggregator.reset();
public void stop() {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
if (isFrameMetricsAggregatorAvailable()) {
runSafelyOnUiThread(() -> frameMetricsAggregator.stop(), "FrameMetricsAggregator.stop");
frameMetricsAggregator.reset();
}
activityMeasurements.clear();
}
activityMeasurements.clear();
}

private void runSafelyOnUiThread(final Runnable runnable, final String tag) {
Expand Down
Loading

0 comments on commit 4a331fa

Please sign in to comment.