Repository navigation
fix(flutter): Await in-flight replay capture on scheduler stop - #3935
Conversation
Scheduler.stop() only awaited the delay timer between runs, not the capture itself, so pause()/stop() could return while a screenshot was still rendering. On Android/Impeller this let a capture's GPU texture release race the Activity's Vulkan surface teardown, aborting in ~TextureVK (#3923). Co-Authored-By: Claude <noreply@anthropic.com>
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Fixes
Dependencies
Internal Changes
🤖 This preview updates automatically when you update the PR. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3935 +/- ##
==========================================
+ Coverage 88.06% 92.69% +4.63%
==========================================
Files 347 114 -233
Lines 12933 4424 -8509
==========================================
- Hits 11389 4101 -7288
+ Misses 1544 323 -1221
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Android Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| dc53d48 | 358.91 ms | 366.17 ms | 7.26 ms |
| d18c337 | 355.25 ms | 342.15 ms | -13.10 ms |
| 6e7d494 | 397.35 ms | 378.91 ms | -18.43 ms |
| 2d34233 | 470.54 ms | 558.90 ms | 88.36 ms |
| 57eccdf | 437.96 ms | 437.94 ms | -0.02 ms |
| e45c0e1 | 447.29 ms | 558.33 ms | 111.04 ms |
| d8f332d | 365.31 ms | 349.98 ms | -15.33 ms |
| b4b7781 | 399.82 ms | 394.26 ms | -5.56 ms |
| 79f6b41 | 469.66 ms | 525.90 ms | 56.24 ms |
| 7701e0e | 361.66 ms | 366.66 ms | 5.00 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| dc53d48 | 14.09 MiB | 15.28 MiB | 1.19 MiB |
| d18c337 | 14.77 MiB | 16.02 MiB | 1.25 MiB |
| 6e7d494 | 13.93 MiB | 15.06 MiB | 1.13 MiB |
| 2d34233 | 6.54 MiB | 7.55 MiB | 1.01 MiB |
| 57eccdf | 14.30 MiB | 15.49 MiB | 1.19 MiB |
| e45c0e1 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
| d8f332d | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| b4b7781 | 14.31 MiB | 15.49 MiB | 1.19 MiB |
| 79f6b41 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
| 7701e0e | 14.31 MiB | 15.49 MiB | 1.19 MiB |
Previous results on branch: lz/sigart-vk
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1f5bd67 | 415.40 ms | 404.56 ms | -10.84 ms |
| 7ea65be | 350.87 ms | 350.58 ms | -0.29 ms |
| 3989d55 | 371.38 ms | 399.09 ms | 27.71 ms |
| ec5779f | 353.59 ms | 358.57 ms | 4.98 ms |
| d297b6e | 440.62 ms | 436.60 ms | -4.01 ms |
| e2b43f3 | 359.67 ms | 357.70 ms | -1.97 ms |
| 005190f | 390.72 ms | 417.40 ms | 26.68 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1f5bd67 | 14.77 MiB | 16.08 MiB | 1.31 MiB |
| 7ea65be | 14.77 MiB | 16.08 MiB | 1.31 MiB |
| 3989d55 | 14.77 MiB | 16.02 MiB | 1.25 MiB |
| ec5779f | 14.77 MiB | 16.08 MiB | 1.31 MiB |
| d297b6e | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| e2b43f3 | 14.77 MiB | 16.08 MiB | 1.31 MiB |
| 005190f | 14.77 MiB | 16.02 MiB | 1.25 MiB |
iOS Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2d42aac | 1245.65 ms | 1255.93 ms | 10.29 ms |
| e200a70 | 1245.37 ms | 1252.35 ms | 6.98 ms |
| a2c8611 | 1253.83 ms | 1246.92 ms | -6.91 ms |
| 17bac94 | 1249.71 ms | 1260.17 ms | 10.46 ms |
| 5b9a0da | 1249.69 ms | 1250.71 ms | 1.03 ms |
| 9b99523 | 1256.06 ms | 1270.33 ms | 14.27 ms |
| 3135a81 | 1258.11 ms | 1247.48 ms | -10.63 ms |
| e2d675d | 1238.48 ms | 1242.76 ms | 4.28 ms |
| 1837f6c | 1255.00 ms | 1255.08 ms | 0.08 ms |
| 6bcdc99 | 1257.16 ms | 1264.96 ms | 7.79 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2d42aac | 5.73 MiB | 6.18 MiB | 466.16 KiB |
| e200a70 | 5.53 MiB | 5.96 MiB | 444.86 KiB |
| a2c8611 | 5.66 MiB | 6.10 MiB | 452.11 KiB |
| 17bac94 | 5.91 MiB | 6.35 MiB | 450.93 KiB |
| 5b9a0da | 5.53 MiB | 5.96 MiB | 444.32 KiB |
| 9b99523 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 3135a81 | 5.73 MiB | 6.17 MiB | 453.76 KiB |
| e2d675d | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 1837f6c | 5.91 MiB | 6.35 MiB | 451.30 KiB |
| 6bcdc99 | 5.53 MiB | 6.00 MiB | 479.95 KiB |
Previous results on branch: lz/sigart-vk
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1f5bd67 | 1255.26 ms | 1261.20 ms | 5.94 ms |
| ec5779f | 1242.96 ms | 1241.33 ms | -1.63 ms |
| 3989d55 | 1260.12 ms | 1261.05 ms | 0.92 ms |
| d297b6e | 1244.51 ms | 1247.76 ms | 3.25 ms |
| 005190f | 1236.06 ms | 1249.98 ms | 13.92 ms |
| 7ea65be | 1252.07 ms | 1256.60 ms | 4.53 ms |
| e2b43f3 | 1250.00 ms | 1256.35 ms | 6.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1f5bd67 | 5.91 MiB | 6.35 MiB | 451.35 KiB |
| ec5779f | 5.91 MiB | 6.35 MiB | 451.30 KiB |
| 3989d55 | 5.91 MiB | 6.35 MiB | 450.94 KiB |
| d297b6e | 5.83 MiB | 6.28 MiB | 457.79 KiB |
| 005190f | 5.91 MiB | 6.35 MiB | 450.93 KiB |
| 7ea65be | 5.91 MiB | 6.35 MiB | 451.25 KiB |
| e2b43f3 | 5.91 MiB | 6.35 MiB | 451.24 KiB |
buenaflor
left a comment
There was a problem hiding this comment.
LGTM, but before I approve I wanna test it on my physical Android device
|
Sry for the delay, I'll properly test it tomorrow |
No rush, I ordered an android device to help with tests and it should arrive on day 20 |
A stalled capture would previously hang stop()/pause()/close() indefinitely. Cap the wait and drop the stale callback reference on timeout so the scheduler can resume on the next start(). Co-Authored-By: Claude <noreply@anthropic.com>
buenaflor
left a comment
There was a problem hiding this comment.
Pls check if the AI reviews are legitimate
A callback future abandoned by stop()'s timeout could still fire its whenComplete listener later and re-schedule a frame callback after a restart, causing overlapping replay captures. Flagged independently by Cursor Bugbot and Sentry's AI reviewer on this PR. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWT754cVGtLfCXUnGFjSMv
They were valid, fixed them on 7ea65be |
|
I am not 100% sure to be honest if this is actually mitigating / fixing the core issue. Check my comment in the issue, I think this is a Flutter bug |
|
@buenaflor It may be unrelated to that flutter bug, but still, would you rather me to block this PR/issue until a fix on Flutter is released? |
|
@lucas-zimerman okay I think this PR should land but I'm unsure if 2s timeout is too aggressive |
|
Do you think it also makes sense to check if we can capture at all? e.g from this: _scheduler = Scheduler(
frameDuration,
(_) => capture(_onImageCaptured),
_addPostFrameCallback,
);to this: // in scheduled_recorder.dart
_scheduler = Scheduler(
frameDuration,
(_) async {
if (!_canStartCapture) {
internalLogger.debug(() =>
'$logName: skipping capture, app is not resumed.'));
return;
}
await capture(_onImageCaptured);
},
_addPostFrameCallback,
);
/// The engine's surface can be torn down as soon as the app leaves
/// [AppLifecycleState.resumed]. A capture started after that point can
/// outlive the surface backing its GPU texture, and releasing the texture
/// against a dead Vulkan context aborts in the driver (#3923).
///
/// Captures already in flight when the transition happens are handled by
/// [Scheduler.stop]; this only prevents new ones.
bool get _canStartCapture {
final state = options.bindingUtils.instance?.lifecycleState;
// null until the first lifecycle message arrives, i.e. during startup,
// where the app is foreground by definition.
return state == null || state == AppLifecycleState.resumed;
} |
The engine's surface can be torn down as soon as the app leaves AppLifecycleState.resumed. A capture started after that point can outlive the surface backing its GPU texture, aborting the driver on release (#3923) - the same crash class this PR's Scheduler.stop() fix addresses, but for captures that start after backgrounding rather than ones already in flight when it happens. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3989d55. Configure here.
A stop() that raced the pending scheduling timer bumped the generation guard before _runAfterNextFrame() attached its whenComplete() listener, so a stalled callback completing later was no longer recognized as stale and could re-trigger an extra capture. Tie the generation to when the callback started instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

📜 Description
Scheduler.stop()only awaited the delay timer between scheduled runs, not the in-flight capture itself.ScheduledScreenshotRecorder.pause()/stop()could therefore return while a screenshot capture was still rendering, disposing its image, and releasing its GPU texture on the engine's message loop.Scheduler.stop()now also awaits the currently in-flight capture (_runningCallback) before returning.💡 Motivation and Context
Fixes #3923. A production user hit a native
SIGABRT~3s afterMainActivitywas destroyed, in~TextureVKduring a message-loop task flush on Android/Impeller (Vulkan backend). The crash was attributed to the session replay recorder: a screenshot capture that was still in flight when the app was backgrounded/destroyed could complete and post its GPU texture release after the Activity's Vulkan surface had already been torn down, aborting inside the driver.pause()/stop()are invoked from native lifecycle callbacks and are awaited there, but the underlyingScheduler.stop()wasn't actually waiting for the capture to finish — only for a delay timer that's a loose proxy for it. This closes that gap so GPU resources are released while the surface is still alive.💚 How did you test it?
test/replay/scheduler_test.dart) that reproduces the race: it starts a capture gated on an uncompletedCompleter, callsstop(), and assertsstop()does not resolve until the capture completes. This test fails against the old implementation and passes with the fix.test/replay/suite plusflutter analyze— all green.force-stopwhile a replay segment was actively encoding. No crashes, ANRs, or hangs;pause()returned promptly in all cases.TextureVKpath from the report can't be reproduced on an emulator — this fix was verified functionally rather than via a native crash repro. Atm I don't have an Android device with vulkan support for testing this.📝 Checklist
sendDefaultPiiis enabled🔮 Next steps
None planned; this is a scoped fix to
Scheduler.stop().