Skip to content

fix(gestures): Thread-safe SentryGestureDetector with per-gesture VelocityTracker recycle#5301

Open
romtsn wants to merge 2 commits intomainfrom
rz/fix/gesture-lifecycle
Open

fix(gestures): Thread-safe SentryGestureDetector with per-gesture VelocityTracker recycle#5301
romtsn wants to merge 2 commits intomainfrom
rz/fix/gesture-lifecycle

Conversation

@romtsn
Copy link
Copy Markdown
Member

@romtsn romtsn commented Apr 17, 2026

📜 Description

Three related fixes to SentryGestureDetector, the lightweight gesture detector introduced in #5138:

  1. Recycle VelocityTracker per gesture. On every ACTION_UP / ACTION_CANCEL the pooled native tracker is recycled and nulled, and re-obtained lazily on the next ACTION_DOWN. Matches framework GestureDetector behavior. Previously the tracker was held for the detector's entire lifetime, keeping one pooled instance out of circulation.
  2. Merge endGesture() + release() into a single recycle(). They did the same thing after the fix above.
  3. Guard onTouchEvent and recycle with an AutoClosableReentrantLock. Sentry.close() is not constrained to the UI thread, so SentryWindowCallback.stopTracking()SentryGestureDetector.recycle() can race UI-thread touch dispatch. Without synchronization that causes use-after-recycle on the native MotionEvent / VelocityTracker pools — the bg thread can return a pooled instance that UI is still calling addMovement() / computeCurrentVelocity() on, and unrelated callers that re-obtain that slot get a corrupted state. recycle() captures the native handles under the lock and performs the JNI recycle() calls outside, keeping the bg thread's critical section to a pointer swap.

SentryWindowCallback is updated for the release()recycle() rename.

💡 Motivation and Context

Before #5138, the detector delegated to GestureDetectorCompat which didn't own pooled native resources directly, so there was no thread-safety hazard at this layer. The new lightweight detector manages MotionEvent.obtain/recycle and VelocityTracker.obtain/recycle itself, which reopens the race described above once Sentry.close() is called off the main thread.

Holding a VelocityTracker across gestures also deviates from framework behavior and unnecessarily ties up a pooled instance.

💚 How did you test it?

  • Existing SentryGestureDetectorTest suite (tap / scroll / fling / cancel / multi-touch / sequential gestures) still passes.
  • New test: recycle mid-gesture - subsequent gesture still fires onSingleTapUp — calls recycle() between ACTION_DOWN and a fresh gesture to verify the lazy re-obtain path and that an external mid-gesture teardown doesn't leave the detector broken.
  • ./gradlew :sentry-android-core:testDebugUnitTest --tests "*SentryGestureDetectorTest*" → 11/11 pass.
  • ./gradlew spotlessApply apiDump — no public API changes.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

…th 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>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 17, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • (gestures) Thread-safe SentryGestureDetector with per-gesture VelocityTracker recycle by romtsn in #5301

🤖 This preview updates automatically when you update the PR.

@sentry
Copy link
Copy Markdown

sentry bot commented Apr 17, 2026

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.39.1 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions
Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 312.41 ms 356.02 ms 43.61 ms
Size 0 B 0 B 0 B

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d15471f 310.66 ms 368.19 ms 57.53 ms
9054d65 330.94 ms 403.24 ms 72.30 ms
b77456b 393.26 ms 441.10 ms 47.84 ms
96449e8 361.30 ms 423.39 ms 62.09 ms
ee747ae 357.79 ms 421.84 ms 64.05 ms
a416a65 295.53 ms 373.74 ms 78.21 ms
ab8a72d 316.24 ms 356.38 ms 40.14 ms
1564554 323.06 ms 336.68 ms 13.62 ms
b3d8889 371.69 ms 432.96 ms 61.26 ms
d15471f 315.20 ms 370.22 ms 55.02 ms

App size

Revision Plain With Sentry Diff
d15471f 1.58 MiB 2.13 MiB 559.54 KiB
9054d65 1.58 MiB 2.29 MiB 723.38 KiB
b77456b 1.58 MiB 2.12 MiB 548.11 KiB
96449e8 1.58 MiB 2.11 MiB 539.35 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
a416a65 1.58 MiB 2.12 MiB 555.26 KiB
ab8a72d 1.58 MiB 2.12 MiB 551.55 KiB
1564554 1.58 MiB 2.20 MiB 635.33 KiB
b3d8889 1.58 MiB 2.10 MiB 535.06 KiB
d15471f 1.58 MiB 2.13 MiB 559.54 KiB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant