Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 2fcda64

Browse filesBrowse 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
Copy full SHA for 2fcda64

4 files changed

+119-87Lines changed: 119 additions & 87 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎CHANGELOG.md‎

Copy file name to clipboardExpand all lines: CHANGELOG.md
+2-1Lines changed: 2 additions & 1 deletion
  • Display the source diff
  • Display the rich diff
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

Collapse file

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

Copy file name to clipboardExpand all lines: sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureDetector.java
+88-85Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java
+1-1Lines 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() {
Collapse file

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

Copy file name to clipboardExpand all lines: sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureDetectorTest.kt
+28Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.