Skip to content

Commit 2fcda64

Browse files
romtsnclaude
romtsn
and
claude
authored
fix(gestures): Thread-safe SentryGestureDetector with per-gesture VelocityTracker recycle (#5301)
* fix(gestures): Recycle VelocityTracker per gesture and guard state with a lock Recycle VelocityTracker on every ACTION_UP/ACTION_CANCEL instead of only when the detector is torn down, so the pooled native tracker isn't held across gestures (matches Android's framework GestureDetector behavior). Merges endGesture() and release() into a single recycle() method. Guard onTouchEvent and recycle with an AutoClosableReentrantLock — SentryWindowCallback.stopTracking() can be invoked from a bg thread via Sentry.close(), which would otherwise race with the UI thread's touch dispatch and cause use-after-recycle on the native MotionEvent/VelocityTracker pools. recycle() captures the native handles under the lock and performs the JNI recycle() calls outside it to keep the bg thread's critical section to a pointer swap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * changelog --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 952b180 commit 2fcda64

4 files changed

Lines changed: 119 additions & 87 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
- Fix `NoSuchMethodError` for `LayoutCoordinates.localBoundingBoxOf$default` on Compose touch dispatch with AGP 8.13 and `minSdk < 24` ([#5302](https://github.com/getsentry/sentry-java/pull/5302))
88
- Fix reporting OkHttp's synthetic 504 "Unsatisfiable Request" responses as errors for `CacheControl.FORCE_CACHE` cache misses ([#5299](https://github.com/getsentry/sentry-java/pull/5299))
9+
- Make `SentryGestureDetector` thread-safe and recycle `VelocityTracker` per gesture ([#5301](https://github.com/getsentry/sentry-java/pull/5301))
10+
- Fix duplicate `ui.click` breadcrumbs when another `Window.Callback` wraps `SentryWindowCallback` ([#5300](https://github.com/getsentry/sentry-java/pull/5300))
911

1012
### Dependencies
1113

@@ -27,7 +29,6 @@
2729
### Fixes
2830

2931
- Fix ANR caused by `GestureDetectorCompat` Handler/MessageQueue lock contention in `SentryWindowCallback` ([#5138](https://github.com/getsentry/sentry-java/pull/5138))
30-
- Fix duplicate `ui.click` breadcrumbs when another `Window.Callback` wraps `SentryWindowCallback` ([#5300](https://github.com/getsentry/sentry-java/pull/5300))
3132

3233
### Internal
3334

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureDetector.java‎

Lines changed: 88 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
import android.view.MotionEvent;
66
import android.view.VelocityTracker;
77
import android.view.ViewConfiguration;
8+
import io.sentry.ISentryLifecycleToken;
9+
import io.sentry.util.AutoClosableReentrantLock;
810
import org.jetbrains.annotations.ApiStatus;
911
import org.jetbrains.annotations.NotNull;
1012
import org.jetbrains.annotations.Nullable;
@@ -35,6 +37,8 @@ public final class SentryGestureDetector {
3537
private @Nullable MotionEvent currentDownEvent;
3638
private @Nullable VelocityTracker velocityTracker;
3739

40+
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
41+
3842
SentryGestureDetector(
3943
final @NotNull Context context, final @NotNull GestureDetector.OnGestureListener listener) {
4044
this.listener = listener;
@@ -46,102 +50,101 @@ public final class SentryGestureDetector {
4650
}
4751

4852
void onTouchEvent(final @NotNull MotionEvent event) {
49-
final int action = event.getActionMasked();
53+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
54+
final int action = event.getActionMasked();
55+
56+
if (velocityTracker == null) {
57+
velocityTracker = VelocityTracker.obtain();
58+
}
59+
velocityTracker.addMovement(event);
60+
61+
switch (action) {
62+
case MotionEvent.ACTION_DOWN:
63+
downX = event.getX();
64+
downY = event.getY();
65+
lastX = downX;
66+
lastY = downY;
67+
isInTapRegion = true;
68+
ignoreUpEvent = false;
69+
70+
if (currentDownEvent != null) {
71+
currentDownEvent.recycle();
72+
}
73+
currentDownEvent = MotionEvent.obtain(event);
5074

51-
if (velocityTracker == null) {
52-
velocityTracker = VelocityTracker.obtain();
53-
}
75+
listener.onDown(event);
76+
break;
5477

55-
if (action == MotionEvent.ACTION_DOWN) {
56-
velocityTracker.clear();
57-
}
58-
velocityTracker.addMovement(event);
59-
60-
switch (action) {
61-
case MotionEvent.ACTION_DOWN:
62-
downX = event.getX();
63-
downY = event.getY();
64-
lastX = downX;
65-
lastY = downY;
66-
isInTapRegion = true;
67-
ignoreUpEvent = false;
68-
69-
if (currentDownEvent != null) {
70-
currentDownEvent.recycle();
71-
}
72-
currentDownEvent = MotionEvent.obtain(event);
73-
74-
listener.onDown(event);
75-
break;
76-
77-
case MotionEvent.ACTION_MOVE:
78-
{
79-
final float x = event.getX();
80-
final float y = event.getY();
81-
final float dx = x - downX;
82-
final float dy = y - downY;
83-
final float distanceSquare = (dx * dx) + (dy * dy);
84-
85-
if (distanceSquare > touchSlopSquare) {
86-
final float scrollX = lastX - x;
87-
final float scrollY = lastY - y;
88-
listener.onScroll(currentDownEvent, event, scrollX, scrollY);
89-
isInTapRegion = false;
90-
lastX = x;
91-
lastY = y;
78+
case MotionEvent.ACTION_MOVE:
79+
{
80+
final float x = event.getX();
81+
final float y = event.getY();
82+
final float dx = x - downX;
83+
final float dy = y - downY;
84+
final float distanceSquare = (dx * dx) + (dy * dy);
85+
86+
if (distanceSquare > touchSlopSquare) {
87+
final float scrollX = lastX - x;
88+
final float scrollY = lastY - y;
89+
listener.onScroll(currentDownEvent, event, scrollX, scrollY);
90+
isInTapRegion = false;
91+
lastX = x;
92+
lastY = y;
93+
}
94+
break;
9295
}
96+
97+
case MotionEvent.ACTION_POINTER_DOWN:
98+
// A second finger means this is not a single tap (e.g. pinch-to-zoom).
99+
// Also suppress the UP handler to avoid spurious fling detection when the
100+
// last finger lifts quickly after a pinch — mirrors GestureDetector's
101+
// mIgnoreNextUpEvent / cancelTaps() behavior.
102+
isInTapRegion = false;
103+
ignoreUpEvent = true;
93104
break;
94-
}
95-
96-
case MotionEvent.ACTION_POINTER_DOWN:
97-
// A second finger means this is not a single tap (e.g. pinch-to-zoom).
98-
// Also suppress the UP handler to avoid spurious fling detection when the
99-
// last finger lifts quickly after a pinch — mirrors GestureDetector's
100-
// mIgnoreNextUpEvent / cancelTaps() behavior.
101-
isInTapRegion = false;
102-
ignoreUpEvent = true;
103-
break;
104-
105-
case MotionEvent.ACTION_UP:
106-
if (ignoreUpEvent) {
107-
endGesture();
108-
break;
109-
}
110-
if (isInTapRegion) {
111-
listener.onSingleTapUp(event);
112-
} else {
113-
final int pointerId = event.getPointerId(0);
114-
velocityTracker.computeCurrentVelocity(1000, maximumFlingVelocity);
115-
final float velocityX = velocityTracker.getXVelocity(pointerId);
116-
final float velocityY = velocityTracker.getYVelocity(pointerId);
117-
118-
if (Math.abs(velocityX) > minimumFlingVelocity
119-
|| Math.abs(velocityY) > minimumFlingVelocity) {
120-
listener.onFling(currentDownEvent, event, velocityX, velocityY);
105+
106+
case MotionEvent.ACTION_UP:
107+
if (ignoreUpEvent) {
108+
recycle();
109+
break;
121110
}
122-
}
123-
endGesture();
124-
break;
111+
if (isInTapRegion) {
112+
listener.onSingleTapUp(event);
113+
} else {
114+
final int pointerId = event.getPointerId(0);
115+
velocityTracker.computeCurrentVelocity(1000, maximumFlingVelocity);
116+
final float velocityX = velocityTracker.getXVelocity(pointerId);
117+
final float velocityY = velocityTracker.getYVelocity(pointerId);
118+
119+
if (Math.abs(velocityX) > minimumFlingVelocity
120+
|| Math.abs(velocityY) > minimumFlingVelocity) {
121+
listener.onFling(currentDownEvent, event, velocityX, velocityY);
122+
}
123+
}
124+
recycle();
125+
break;
125126

126-
case MotionEvent.ACTION_CANCEL:
127-
endGesture();
128-
break;
127+
case MotionEvent.ACTION_CANCEL:
128+
recycle();
129+
break;
130+
}
129131
}
130132
}
131133

132-
/** Releases native resources. Call when the detector is no longer needed. */
133-
void release() {
134-
endGesture();
135-
if (velocityTracker != null) {
136-
velocityTracker.recycle();
134+
void recycle() {
135+
final @Nullable MotionEvent capturedDownEvent;
136+
final @Nullable VelocityTracker capturedVelocityTracker;
137+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
138+
capturedDownEvent = currentDownEvent;
139+
currentDownEvent = null;
140+
capturedVelocityTracker = velocityTracker;
137141
velocityTracker = null;
138142
}
139-
}
140-
141-
private void endGesture() {
142-
if (currentDownEvent != null) {
143-
currentDownEvent.recycle();
144-
currentDownEvent = null;
143+
if (capturedDownEvent != null) {
144+
capturedDownEvent.recycle();
145+
}
146+
if (capturedVelocityTracker != null) {
147+
capturedVelocityTracker.recycle();
145148
}
146149
}
147150
}

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private void handleTouchEvent(final @NotNull MotionEvent motionEvent) {
8181
public void stopTracking() {
8282
inert = true;
8383
gestureListener.stopTracing(SpanStatus.CANCELLED);
84-
gestureDetector.release();
84+
gestureDetector.recycle();
8585
}
8686

8787
public @NotNull Window.Callback getDelegate() {

‎sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureDetectorTest.kt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,34 @@ class SentryGestureDetectorTest {
322322
up2.recycle()
323323
}
324324

325+
@Test
326+
fun `recycle mid-gesture - subsequent gesture still fires onSingleTapUp`() {
327+
val sut = fixture.getSut()
328+
val downTime = SystemClock.uptimeMillis()
329+
330+
// Start a gesture, then simulate stopTracking() racing in mid-gesture.
331+
val down1 = MotionEvent.obtain(downTime, downTime, MotionEvent.ACTION_DOWN, 100f, 100f, 0)
332+
sut.onTouchEvent(down1)
333+
sut.recycle()
334+
335+
verify(fixture.listener).onDown(down1)
336+
337+
// New gesture after recycle — velocityTracker and currentDownEvent should be re-obtained
338+
// lazily and the tap path should work as normal.
339+
val downTime2 = SystemClock.uptimeMillis()
340+
val down2 = MotionEvent.obtain(downTime2, downTime2, MotionEvent.ACTION_DOWN, 200f, 200f, 0)
341+
val up2 = MotionEvent.obtain(downTime2, downTime2 + 50, MotionEvent.ACTION_UP, 200f, 200f, 0)
342+
343+
sut.onTouchEvent(down2)
344+
sut.onTouchEvent(up2)
345+
346+
verify(fixture.listener).onSingleTapUp(up2)
347+
348+
down1.recycle()
349+
down2.recycle()
350+
up2.recycle()
351+
}
352+
325353
@Test
326354
fun `sequential gestures - state resets between tap and scroll`() {
327355
val sut = fixture.getSut()

0 commit comments

Comments
 (0)