Skip to content

Commit e60ca1b

Browse files
romtsnclaude
andcommitted
fix(user-interaction): Restore window callbacks on close and dedup instrumentation via WeakHashMap
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) <noreply@anthropic.com>
1 parent de6a178 commit e60ca1b

File tree

2 files changed

+113
-12
lines changed

2 files changed

+113
-12
lines changed

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

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
import io.sentry.util.Objects;
1919
import java.io.Closeable;
2020
import java.io.IOException;
21+
import java.lang.ref.WeakReference;
22+
import java.util.ArrayList;
23+
import java.util.WeakHashMap;
2124
import org.jetbrains.annotations.NotNull;
2225
import org.jetbrains.annotations.Nullable;
2326

@@ -30,6 +33,16 @@ public final class UserInteractionIntegration
3033

3134
private final boolean isAndroidxLifecycleAvailable;
3235

36+
// WeakReference value, because the callback chain strongly references the wrapper — a strong
37+
// value would prevent the window from ever being GC'd.
38+
//
39+
// All access must be guarded by wrappedWindowsLock — lifecycle callbacks fire on the main
40+
// thread, but close() may be called from a background thread (e.g. Sentry.close()).
41+
private final @NotNull WeakHashMap<Window, WeakReference<SentryWindowCallback>> wrappedWindows =
42+
new WeakHashMap<>();
43+
44+
private final @NotNull Object wrappedWindowsLock = new Object();
45+
3346
public UserInteractionIntegration(
3447
final @NotNull Application application, final @NotNull io.sentry.util.LoadClass classLoader) {
3548
this.application = Objects.requireNonNull(application, "Application is required");
@@ -47,19 +60,26 @@ private void startTracking(final @NotNull Activity activity) {
4760
}
4861

4962
if (scopes != null && options != null) {
63+
synchronized (wrappedWindowsLock) {
64+
final @Nullable WeakReference<SentryWindowCallback> cached = wrappedWindows.get(window);
65+
if (cached != null && cached.get() != null) {
66+
return;
67+
}
68+
}
69+
5070
Window.Callback delegate = window.getCallback();
5171
if (delegate == null) {
5272
delegate = new NoOpWindowCallback();
5373
}
5474

55-
if (delegate instanceof SentryWindowCallback) {
56-
// already instrumented
57-
return;
58-
}
59-
6075
final SentryGestureListener gestureListener =
6176
new SentryGestureListener(activity, scopes, options);
62-
window.setCallback(new SentryWindowCallback(delegate, activity, gestureListener, options));
77+
final SentryWindowCallback wrapper =
78+
new SentryWindowCallback(delegate, activity, gestureListener, options);
79+
window.setCallback(wrapper);
80+
synchronized (wrappedWindowsLock) {
81+
wrappedWindows.put(window, new WeakReference<>(wrapper));
82+
}
6383
}
6484
}
6585

@@ -71,7 +91,10 @@ private void stopTracking(final @NotNull Activity activity) {
7191
}
7292
return;
7393
}
94+
unwrapWindow(window);
95+
}
7496

97+
private void unwrapWindow(final @NotNull Window window) {
7598
final Window.Callback current = window.getCallback();
7699
if (current instanceof SentryWindowCallback) {
77100
((SentryWindowCallback) current).stopTracking();
@@ -80,6 +103,22 @@ private void stopTracking(final @NotNull Activity activity) {
80103
} else {
81104
window.setCallback(((SentryWindowCallback) current).getDelegate());
82105
}
106+
synchronized (wrappedWindowsLock) {
107+
wrappedWindows.remove(window);
108+
}
109+
return;
110+
}
111+
112+
// Another wrapper (e.g. Session Replay) sits on top of ours — cutting it out of the chain
113+
// would break its instrumentation, so we leave the chain alone and only release our
114+
// resources. The inert wrapper gets GC'd when the window is destroyed.
115+
final @Nullable SentryWindowCallback ours;
116+
synchronized (wrappedWindowsLock) {
117+
final @Nullable WeakReference<SentryWindowCallback> cached = wrappedWindows.get(window);
118+
ours = cached != null ? cached.get() : null;
119+
}
120+
if (ours != null) {
121+
ours.stopTracking();
83122
}
84123
}
85124

@@ -146,6 +185,21 @@ public void register(@NotNull IScopes scopes, @NotNull SentryOptions options) {
146185
public void close() throws IOException {
147186
application.unregisterActivityLifecycleCallbacks(this);
148187

188+
// Restore original callbacks so a subsequent Sentry.init() starts from a clean chain instead
189+
// of wrapping on top of our orphaned callback.
190+
final ArrayList<Window> snapshot;
191+
synchronized (wrappedWindowsLock) {
192+
snapshot = new ArrayList<>(wrappedWindows.keySet());
193+
}
194+
for (final Window window : snapshot) {
195+
if (window != null) {
196+
unwrapWindow(window);
197+
}
198+
}
199+
synchronized (wrappedWindowsLock) {
200+
wrappedWindows.clear();
201+
}
202+
149203
if (options != null) {
150204
options.getLogger().log(SentryLevel.DEBUG, "UserInteractionIntegration removed.");
151205
}

sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import kotlin.test.BeforeTest
1414
import kotlin.test.Test
1515
import kotlin.test.assertIs
1616
import kotlin.test.assertIsNot
17-
import kotlin.test.assertNotEquals
1817
import kotlin.test.assertSame
1918
import org.junit.runner.RunWith
2019
import org.mockito.kotlin.any
@@ -149,15 +148,59 @@ class UserInteractionIntegrationTest {
149148
}
150149

151150
@Test
152-
fun `does not instrument if the callback is already ours`() {
153-
val existingCallback =
154-
SentryWindowCallback(NoOpWindowCallback(), fixture.activity, mock(), mock())
155-
val sut = fixture.getSut(existingCallback)
151+
fun `does not double-wrap when another callback wraps SentryWindowCallback`() {
152+
val sut = fixture.getSut()
153+
sut.register(fixture.scopes, fixture.options)
154+
155+
sut.onActivityResumed(fixture.activity)
156+
val sentryCallback = fixture.window.callback
157+
assertIs<SentryWindowCallback>(sentryCallback)
158+
159+
val outerWrapper = WrapperCallback(sentryCallback)
160+
fixture.window.callback = outerWrapper
161+
162+
sut.onActivityPaused(fixture.activity)
163+
sut.onActivityResumed(fixture.activity)
164+
165+
assertSame(outerWrapper, fixture.window.callback)
166+
}
167+
168+
@Test
169+
fun `close unwraps windows so re-init does not double-wrap`() {
170+
val mockCallback = mock<Window.Callback>()
171+
fixture.window.callback = mockCallback
156172

173+
val sutA = fixture.getSut()
174+
sutA.register(fixture.scopes, fixture.options)
175+
sutA.onActivityResumed(fixture.activity)
176+
assertIs<SentryWindowCallback>(fixture.window.callback)
177+
178+
sutA.close()
179+
assertSame(mockCallback, fixture.window.callback)
180+
181+
val sutB = UserInteractionIntegration(fixture.application, fixture.loadClass)
182+
sutB.register(fixture.scopes, fixture.options)
183+
sutB.onActivityResumed(fixture.activity)
184+
185+
val newWrapper = fixture.window.callback
186+
assertIs<SentryWindowCallback>(newWrapper)
187+
assertSame(mockCallback, newWrapper.delegate)
188+
}
189+
190+
@Test
191+
fun `paused with another wrapper on top does not cut it out of the chain`() {
192+
val sut = fixture.getSut()
157193
sut.register(fixture.scopes, fixture.options)
194+
158195
sut.onActivityResumed(fixture.activity)
196+
val sentryCallback = fixture.window.callback as SentryWindowCallback
197+
198+
val outerWrapper = WrapperCallback(sentryCallback)
199+
fixture.window.callback = outerWrapper
159200

160-
assertNotEquals(existingCallback, (fixture.window.callback as SentryWindowCallback).delegate)
201+
sut.onActivityPaused(fixture.activity)
202+
203+
assertSame(outerWrapper, fixture.window.callback)
161204
}
162205

163206
@Test
@@ -205,3 +248,7 @@ class UserInteractionIntegrationTest {
205248
private class EmptyActivity : Activity(), LifecycleOwner {
206249
override val lifecycle: Lifecycle = mock<Lifecycle>()
207250
}
251+
252+
/** Simulates a third-party callback wrapper (e.g. Session Replay's FixedWindowCallback). */
253+
private open class WrapperCallback(@JvmField val delegate: Window.Callback) :
254+
Window.Callback by delegate

0 commit comments

Comments
 (0)