From e60ca1b5f2c5e2be40ac0687d0d951c7fe69fbda Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 17 Apr 2026 10:56:40 +0200 Subject: [PATCH 1/4] fix(user-interaction): Restore window callbacks on close and dedup instrumentation via WeakHashMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track wrapped windows in a thread-safe WeakHashMap so close() can restore each window's original callback chain, preventing an orphaned SentryWindowCallback from persisting after Sentry.close(). Also handle the case where another wrapper (e.g. Session Replay) has been installed on top of ours — we skip chain mutation but still invoke stopTracking() to release resources. Guards against racing lifecycle callbacks (main thread) and close() (possibly bg thread). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../core/UserInteractionIntegration.java | 66 +++++++++++++++++-- .../core/UserInteractionIntegrationTest.kt | 59 +++++++++++++++-- 2 files changed, 113 insertions(+), 12 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java index c0dd3f9eb71..1af38878b37 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java @@ -18,6 +18,9 @@ import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; +import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.WeakHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -30,6 +33,16 @@ public final class UserInteractionIntegration private final boolean isAndroidxLifecycleAvailable; + // WeakReference value, because the callback chain strongly references the wrapper — a strong + // value would prevent the window from ever being GC'd. + // + // All access must be guarded by wrappedWindowsLock — lifecycle callbacks fire on the main + // thread, but close() may be called from a background thread (e.g. Sentry.close()). + private final @NotNull WeakHashMap> wrappedWindows = + new WeakHashMap<>(); + + private final @NotNull Object wrappedWindowsLock = new Object(); + public UserInteractionIntegration( final @NotNull Application application, final @NotNull io.sentry.util.LoadClass classLoader) { this.application = Objects.requireNonNull(application, "Application is required"); @@ -47,19 +60,26 @@ private void startTracking(final @NotNull Activity activity) { } if (scopes != null && options != null) { + synchronized (wrappedWindowsLock) { + final @Nullable WeakReference cached = wrappedWindows.get(window); + if (cached != null && cached.get() != null) { + return; + } + } + Window.Callback delegate = window.getCallback(); if (delegate == null) { delegate = new NoOpWindowCallback(); } - if (delegate instanceof SentryWindowCallback) { - // already instrumented - return; - } - final SentryGestureListener gestureListener = new SentryGestureListener(activity, scopes, options); - window.setCallback(new SentryWindowCallback(delegate, activity, gestureListener, options)); + final SentryWindowCallback wrapper = + new SentryWindowCallback(delegate, activity, gestureListener, options); + window.setCallback(wrapper); + synchronized (wrappedWindowsLock) { + wrappedWindows.put(window, new WeakReference<>(wrapper)); + } } } @@ -71,7 +91,10 @@ private void stopTracking(final @NotNull Activity activity) { } return; } + unwrapWindow(window); + } + private void unwrapWindow(final @NotNull Window window) { final Window.Callback current = window.getCallback(); if (current instanceof SentryWindowCallback) { ((SentryWindowCallback) current).stopTracking(); @@ -80,6 +103,22 @@ private void stopTracking(final @NotNull Activity activity) { } else { window.setCallback(((SentryWindowCallback) current).getDelegate()); } + synchronized (wrappedWindowsLock) { + wrappedWindows.remove(window); + } + return; + } + + // Another wrapper (e.g. Session Replay) sits on top of ours — cutting it out of the chain + // would break its instrumentation, so we leave the chain alone and only release our + // resources. The inert wrapper gets GC'd when the window is destroyed. + final @Nullable SentryWindowCallback ours; + synchronized (wrappedWindowsLock) { + final @Nullable WeakReference cached = wrappedWindows.get(window); + ours = cached != null ? cached.get() : null; + } + if (ours != null) { + ours.stopTracking(); } } @@ -146,6 +185,21 @@ public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) { public void close() throws IOException { application.unregisterActivityLifecycleCallbacks(this); + // Restore original callbacks so a subsequent Sentry.init() starts from a clean chain instead + // of wrapping on top of our orphaned callback. + final ArrayList snapshot; + synchronized (wrappedWindowsLock) { + snapshot = new ArrayList<>(wrappedWindows.keySet()); + } + for (final Window window : snapshot) { + if (window != null) { + unwrapWindow(window); + } + } + synchronized (wrappedWindowsLock) { + wrappedWindows.clear(); + } + if (options != null) { options.getLogger().log(SentryLevel.DEBUG, "UserInteractionIntegration removed."); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt index f558841e6f5..bb28fa1b597 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt @@ -14,7 +14,6 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertIs import kotlin.test.assertIsNot -import kotlin.test.assertNotEquals import kotlin.test.assertSame import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -149,15 +148,59 @@ class UserInteractionIntegrationTest { } @Test - fun `does not instrument if the callback is already ours`() { - val existingCallback = - SentryWindowCallback(NoOpWindowCallback(), fixture.activity, mock(), mock()) - val sut = fixture.getSut(existingCallback) + fun `does not double-wrap when another callback wraps SentryWindowCallback`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + sut.onActivityResumed(fixture.activity) + val sentryCallback = fixture.window.callback + assertIs(sentryCallback) + + val outerWrapper = WrapperCallback(sentryCallback) + fixture.window.callback = outerWrapper + + sut.onActivityPaused(fixture.activity) + sut.onActivityResumed(fixture.activity) + + assertSame(outerWrapper, fixture.window.callback) + } + + @Test + fun `close unwraps windows so re-init does not double-wrap`() { + val mockCallback = mock() + fixture.window.callback = mockCallback + val sutA = fixture.getSut() + sutA.register(fixture.scopes, fixture.options) + sutA.onActivityResumed(fixture.activity) + assertIs(fixture.window.callback) + + sutA.close() + assertSame(mockCallback, fixture.window.callback) + + val sutB = UserInteractionIntegration(fixture.application, fixture.loadClass) + sutB.register(fixture.scopes, fixture.options) + sutB.onActivityResumed(fixture.activity) + + val newWrapper = fixture.window.callback + assertIs(newWrapper) + assertSame(mockCallback, newWrapper.delegate) + } + + @Test + fun `paused with another wrapper on top does not cut it out of the chain`() { + val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) + sut.onActivityResumed(fixture.activity) + val sentryCallback = fixture.window.callback as SentryWindowCallback + + val outerWrapper = WrapperCallback(sentryCallback) + fixture.window.callback = outerWrapper - assertNotEquals(existingCallback, (fixture.window.callback as SentryWindowCallback).delegate) + sut.onActivityPaused(fixture.activity) + + assertSame(outerWrapper, fixture.window.callback) } @Test @@ -205,3 +248,7 @@ class UserInteractionIntegrationTest { private class EmptyActivity : Activity(), LifecycleOwner { override val lifecycle: Lifecycle = mock() } + +/** Simulates a third-party callback wrapper (e.g. Session Replay's FixedWindowCallback). */ +private open class WrapperCallback(@JvmField val delegate: Window.Callback) : + Window.Callback by delegate From 792d200cc71600b2c9676748d1e50332f957667c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 17 Apr 2026 10:56:47 +0200 Subject: [PATCH 2/4] fix(replay): Track wrapped windows and inert buried recorders on stop Track wrapped windows in a WeakHashMap so GestureRecorder skips re-wrapping already-instrumented windows and can locate its own recorder even when another wrapper (e.g. UserInteractionIntegration) has been installed on top of it. When our wrapper is buried in the callback chain, inert() it instead of mutating the chain so unrelated instrumentation isn't broken; the next replay session wraps on top with a fresh active recorder. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../replay/gestures/GestureRecorder.kt | 43 +++++++++++++++--- .../replay/gestures/GestureRecorderTest.kt | 44 ++++++++++++++++--- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt index 945a0be5156..cee75fb06c0 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/gestures/GestureRecorder.kt @@ -11,6 +11,7 @@ import io.sentry.android.replay.phoneWindow import io.sentry.android.replay.util.FixedWindowCallback import io.sentry.util.AutoClosableReentrantLock import java.lang.ref.WeakReference +import java.util.WeakHashMap internal class GestureRecorder( private val options: SentryOptions, @@ -19,6 +20,11 @@ internal class GestureRecorder( private val rootViews = ArrayList>() private val rootViewsLock = AutoClosableReentrantLock() + // WeakReference value, because the callback chain strongly references the wrapper — a strong + // value would prevent the window from ever being GC'd. + private val wrappedWindows = WeakHashMap>() + private val wrappedWindowsLock = AutoClosableReentrantLock() + override fun onRootViewsChanged(root: View, added: Boolean) { rootViewsLock.acquire().use { if (added) { @@ -45,10 +51,16 @@ internal class GestureRecorder( return } - val delegate = window.callback - if (delegate !is SentryReplayGestureRecorder) { - window.callback = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate) + wrappedWindowsLock.acquire().use { + if (wrappedWindows[window]?.get() != null) { + return + } } + + val delegate = window.callback + val wrapper = SentryReplayGestureRecorder(options, touchRecorderCallback, delegate) + window.callback = wrapper + wrappedWindowsLock.acquire().use { wrappedWindows[window] = WeakReference(wrapper) } } private fun View.stopGestureTracking() { @@ -60,14 +72,25 @@ internal class GestureRecorder( val callback = window.callback if (callback is SentryReplayGestureRecorder) { - val delegate = callback.delegate - window.callback = delegate + window.callback = callback.delegate + wrappedWindowsLock.acquire().use { wrappedWindows.remove(window) } + return + } + + // Another wrapper (e.g. UserInteractionIntegration) sits on top of ours — cutting it out of + // the chain would break its instrumentation, so we inert our buried wrapper instead. The + // next replay session will then wrap on top with a fresh active instance. + val ours: SentryReplayGestureRecorder? + wrappedWindowsLock.acquire().use { + ours = wrappedWindows[window]?.get() + wrappedWindows.remove(window) } + ours?.inert() } internal class SentryReplayGestureRecorder( private val options: SentryOptions, - private val touchRecorderCallback: TouchRecorderCallback?, + @Volatile private var touchRecorderCallback: TouchRecorderCallback?, delegate: Window.Callback?, ) : FixedWindowCallback(delegate) { override fun dispatchTouchEvent(event: MotionEvent?): Boolean { @@ -83,6 +106,14 @@ internal class GestureRecorder( } return super.dispatchTouchEvent(event) } + + /** + * Turns this wrapper into a passthrough when it can't be removed from the chain (another + * wrapper sits on top). Subsequent dispatches only delegate, skipping the recorder callback. + */ + fun inert() { + touchRecorderCallback = null + } } } diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt index bf3f9cb8443..6f5a02b54c7 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/gestures/GestureRecorderTest.kt @@ -5,6 +5,7 @@ import android.app.Activity import android.os.Bundle import android.view.MotionEvent import android.view.View +import android.view.Window import android.widget.LinearLayout import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.SentryOptions @@ -14,6 +15,7 @@ import io.sentry.android.replay.phoneWindow import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertSame import kotlin.test.assertTrue import org.junit.runner.RunWith import org.robolectric.Robolectric @@ -37,17 +39,44 @@ class GestureRecorderTest { } @Test - fun `when new window added and window callback is already wrapped, does not wrap it again`() { + fun `does not double-wrap when root is added twice and another callback wraps on top`() { val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() val gestureRecorder = fixture.getSut() - activity.root.phoneWindow?.callback = SentryReplayGestureRecorder(fixture.options, null, null) gestureRecorder.onRootViewsChanged(activity.root, true) + val ourWrapper = activity.root.phoneWindow?.callback as SentryReplayGestureRecorder - assertFalse( - (activity.root.phoneWindow?.callback as SentryReplayGestureRecorder).delegate - is SentryReplayGestureRecorder - ) + val outer = WrapperCallback(ourWrapper) + activity.root.phoneWindow?.callback = outer + + gestureRecorder.onRootViewsChanged(activity.root, true) + + assertSame(outer, activity.root.phoneWindow?.callback) + } + + @Test + fun `when stopped with another wrapper on top, inerts the buried recorder`() { + var called = false + val activity = Robolectric.buildActivity(TestActivity::class.java).setup().get() + val gestureRecorder = + fixture.getSut( + touchRecorderCallback = + object : TouchRecorderCallback { + override fun onTouchEvent(event: MotionEvent) { + called = true + } + } + ) + + gestureRecorder.onRootViewsChanged(activity.root, true) + val ourWrapper = activity.root.phoneWindow?.callback as SentryReplayGestureRecorder + activity.root.phoneWindow?.callback = WrapperCallback(ourWrapper) + + gestureRecorder.onRootViewsChanged(activity.root, false) + + val motionEvent = MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 0f, 0f, 0) + ourWrapper.dispatchTouchEvent(motionEvent) + assertFalse(called) } @Test @@ -109,6 +138,9 @@ class GestureRecorderTest { } } +private open class WrapperCallback(@JvmField val delegate: Window.Callback) : + Window.Callback by delegate + private class TestActivity : Activity() { lateinit var root: View From 8c36cbfed5db1379a84cee8c4827fa282d58dec5 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 17 Apr 2026 12:21:14 +0200 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74219c17d9d..bfad1aef26c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Fix ANR caused by `GestureDetectorCompat` Handler/MessageQueue lock contention in `SentryWindowCallback` ([#5138](https://github.com/getsentry/sentry-java/pull/5138)) +- Fix duplicate `ui.click` breadcrumbs when another `Window.Callback` wraps `SentryWindowCallback` ([#5300](https://github.com/getsentry/sentry-java/pull/5300)) ### Internal From b7674b8191bd56d7b674cc8b7b3d71b18b2e65f3 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 17 Apr 2026 12:59:31 +0200 Subject: [PATCH 4/4] fix(user-interaction): Inert buried SentryWindowCallback and drop its cache entry on stop Two follow-ups to the buried-wrapper path: - stopTracking() now sets an inert flag that short-circuits handleTouchEvent, so a SentryWindowCallback that can't be cut out of the chain stops forwarding events to its gesture detector and listener. Without this, the "stopped" wrapper kept emitting ui.click breadcrumbs, so as soon as a fresh wrapper was installed on top the duplicates came back. - unwrapWindow removes the wrapped window from the tracking map in the buried path too. Previously only the top-of-chain path cleared it, which meant the next startTracking() found a stale (but alive, since the inert wrapper is still referenced by the chain) entry and returned early, permanently losing gesture tracking for that window. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../android/core/UserInteractionIntegration.java | 7 ++++--- .../internal/gestures/SentryWindowCallback.java | 8 ++++++++ .../core/UserInteractionIntegrationTest.kt | 15 ++++++++++----- .../internal/gestures/SentryWindowCallbackTest.kt | 14 ++++++++++++++ 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java index 1af38878b37..0d77625c718 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java @@ -110,11 +110,12 @@ private void unwrapWindow(final @NotNull Window window) { } // Another wrapper (e.g. Session Replay) sits on top of ours — cutting it out of the chain - // would break its instrumentation, so we leave the chain alone and only release our - // resources. The inert wrapper gets GC'd when the window is destroyed. + // would break its instrumentation, so we leave the chain alone and just call stopTracking() + // to release our resources. The upstream wrapper holds a reference to ours, so it'll be + // GC'd whenever that upstream holder is (typically when the window is destroyed). final @Nullable SentryWindowCallback ours; synchronized (wrappedWindowsLock) { - final @Nullable WeakReference cached = wrappedWindows.get(window); + final @Nullable WeakReference cached = wrappedWindows.remove(window); ours = cached != null ? cached.get() : null; } if (ours != null) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java index 557cd4e7a29..e69756e506a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java @@ -19,6 +19,10 @@ public final class SentryWindowCallback extends WindowCallbackAdapter { private final @Nullable SentryOptions options; private final @NotNull MotionEventObtainer motionEventObtainer; + // When we can't be removed from the callback chain (see UserInteractionIntegration), + // stopTracking() flips this so handleTouchEvent short-circuits. + private volatile boolean inert; + public SentryWindowCallback( final @NotNull Window.Callback delegate, final @NotNull Context context, @@ -64,6 +68,9 @@ public boolean dispatchTouchEvent(final @Nullable MotionEvent motionEvent) { } private void handleTouchEvent(final @NotNull MotionEvent motionEvent) { + if (inert) { + return; + } gestureDetector.onTouchEvent(motionEvent); int action = motionEvent.getActionMasked(); if (action == MotionEvent.ACTION_UP) { @@ -72,6 +79,7 @@ private void handleTouchEvent(final @NotNull MotionEvent motionEvent) { } public void stopTracking() { + inert = true; gestureListener.stopTracing(SpanStatus.CANCELLED); gestureDetector.release(); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt index bb28fa1b597..8f20dbb1539 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt @@ -14,6 +14,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertIs import kotlin.test.assertIsNot +import kotlin.test.assertNotSame import kotlin.test.assertSame import org.junit.runner.RunWith import org.mockito.kotlin.any @@ -148,21 +149,25 @@ class UserInteractionIntegrationTest { } @Test - fun `does not double-wrap when another callback wraps SentryWindowCallback`() { + fun `resume after buried pause installs a fresh wrapper on top`() { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) sut.onActivityResumed(fixture.activity) - val sentryCallback = fixture.window.callback - assertIs(sentryCallback) + val originalSentryCallback = fixture.window.callback + assertIs(originalSentryCallback) - val outerWrapper = WrapperCallback(sentryCallback) + // Third-party wraps on top of us mid-activity. + val outerWrapper = WrapperCallback(originalSentryCallback) fixture.window.callback = outerWrapper sut.onActivityPaused(fixture.activity) sut.onActivityResumed(fixture.activity) - assertSame(outerWrapper, fixture.window.callback) + val newTop = fixture.window.callback + assertIs(newTop) + assertNotSame(originalSentryCallback, newTop) + assertSame(outerWrapper, newTop.delegate) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryWindowCallbackTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryWindowCallbackTest.kt index 8afc1b39304..be0438d9345 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryWindowCallbackTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryWindowCallbackTest.kt @@ -82,4 +82,18 @@ class SentryWindowCallbackTest { verify(fixture.gestureDetector, never()).onTouchEvent(any()) } + + @Test + fun `after stopTracking does not forward touches to detector or listener`() { + val event = mock { whenever(it.actionMasked).thenReturn(MotionEvent.ACTION_UP) } + val sut = fixture.getSut() + + sut.stopTracking() + sut.dispatchTouchEvent(event) + + verify(fixture.gestureDetector, never()).onTouchEvent(any()) + verify(fixture.gestureListener, never()).onUp(any()) + // super.dispatchTouchEvent still delegates to the wrapped delegate so the chain keeps working. + verify(fixture.delegate).dispatchTouchEvent(event) + } }