fix(android): harden camera capture hang fix (post-#256 review findings)#257
Merged
Conversation
Both duplicated capture paths (CameraViewfinderDialog.android.kt's takePhotoAndProcess and AndroidCameraProvider.capturePhoto()) already wrap ExifOrientationFixer.fixOrientation in withContext(PlatformDispatcher.IO) inside the widened withTimeout(10_000L) — verified by re-reading both files, this was already fixed by a prior commit on this branch. What was missing was a regression test proving the dispatcher hop, mirroring the existing snapshotSensorData hang test. Also re-ran AndroidCameraFrameSourceTest (QR scan) and CaptureAndImportTest (capture-and-import + withImportTimeout) to confirm no regression — both still green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrCZ7tzBPrWhhj7oJXCXKX
Four-agent parallel review of PR #256 surfaced two real correctness issues, one incomplete fix, and a test-quality gap; addressed all four: - App.kt's CapturePreviewDialog.onSave scope.launch had no try/catch or finally: an uncaught Throwable from imageImportService.import() (or anything downstream of it) would kill the Android process, and even short of a crash would skip the isCaptureImporting reset — reintroducing the exact "stuck forever" bug this PR fixes, one step downstream of the capture dialog. Now wrapped in try/catch(Throwable)/finally. - AndroidCameraProvider.capturePhoto()'s withTimeout(10_000L) didn't actually cover ProcessCameraProvider.getInstance() or bindToLifecycle() — both sat before the timeout block, so a wedged provider/bind could still hang this capture path indefinitely, the acceptance criterion's explicit "both duplicated capture code paths" requirement. Widened the timeout to bound the whole pipeline. - CameraViewfinderDialog.android.kt passed raw Throwable.message (which can now include OOM diagnostic strings, since the outer catch was broadened to Throwable) straight to the user-facing snackbar at four call sites, unlike every other error path in the codebase which routes through DomainError.toUiMessage(). Now logs the raw detail via Logger and surfaces a generic message. Also wrapped the click handler's result.fold in try/catch(Throwable) so a throw from the caller-supplied onCapture/onError/onDismiss callbacks can't crash the process. - Corrected the new dispatcher-hop test's KDoc, which overclaimed that it would catch a regression in the production call sites — it validates the withContext(IO) pattern in isolation, not the wiring. - Added an explicit code comment at both withTimeout(10_000L) sites documenting the known residual risk: Kotlin cancellation is cooperative and cannot preempt a truly HAL-wedged takePicture() or a synchronous BitmapFactory decode mid-call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrCZ7tzBPrWhhj7oJXCXKX
tstapler
force-pushed
the
backlog/stelekit-fix-camera-capture-hang
branch
from
July 24, 2026 22:46
6f27d7a to
3e18def
Compare
tstapler
marked this pull request as ready for review
July 24, 2026 22:47
Contributor
JVM Load Benchmark (Desktop)Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Flamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top allocation hotspots (this PR)`37.2%` byte[]_[k] `7.2%` java.lang.String_[k] `6.6%` int[]_[k] `6.3%` java.util.LinkedHashMap$Entry_[k] `3.9%` java.lang.Object[]_[k]Top CPU hotspots (this PR)`97.6%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0.9%` /tmp/sqlite-3.51.3.0-fcfafaeb-3f19-4187-a745-0237b15f3f22-libsqlitejdbc.so `0.3%` __libc_pwrite `0.1%` fsync `0.1%` SR_handler |
Contributor
Android Load BenchmarkInstrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph. Comparing Graph Load
Interactive Write Latency (during Phase 3)
SAF I/O Overhead (ContentProvider vs direct File read)Measures Binder IPC cost added by ContentResolver per readFile() call.
|
tstapler
pushed a commit
that referenced
this pull request
Jul 25, 2026
🤖 I have created a release *beep* *boop* --- ## [0.71.5](v0.71.4...v0.71.5) (2026-07-24) ### Bug Fixes * **android:** harden camera capture hang fix (post-[#256](#256) review findings) ([#257](#257)) ([6369b87](6369b87)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #256 (merged, shipped in v0.71.4) already fixed the core reported bug — tapping capture hanging forever. This PR is a follow-up hardening pass: a 4-agent parallel code review of #256's changes surfaced two real correctness gaps and a security/UX regression that #256 didn't cover. This PR is rebased directly on
mainand contains only the incremental delta on top of #256 — it does not re-implement anything already shipped.What Changed
App.kt'sCapturePreviewDialog.onSavehad notry/catch/finallyaround itsscope.launchbody. An uncaughtThrowablefromimageImportService.import()(or anything downstream of it) would either crash the Android process (uncaughtThrowableon a plainrememberCoroutineScope()with noCoroutineExceptionHandler— see this repo's own documented failure mode) or, short of a crash, skip theisCaptureImportingreset — reintroducing the exact "stuck forever" bug fix(android): prevent camera capture dialog from hanging forever #256 fixed, one step downstream. Now wrapped intry/catch(Throwable)/finally.AndroidCameraProvider.capturePhoto()'swithTimeout(10_000L)didn't actually coverProcessCameraProvider.getInstance()orbindToLifecycle()— both sat before the timeout block, so a wedged provider/bind could still hang this capture path indefinitely. Widened the timeout to bound the whole pipeline, matching the original bug's acceptance criterion that both duplicated capture paths never hang.CameraViewfinderDialog.android.ktpassed rawThrowable.message(which can includeOutOfMemoryErrordiagnostic strings, since fix(android): prevent camera capture dialog from hanging forever #256 broadened the catch-all toThrowable) straight to the user-facing snackbar at four call sites, unlike every other error path in the codebase which routes throughDomainError.toUiMessage(). Now logs the raw detail and surfaces a generic message. Also wrapped the click handler'sresult.foldintry/catch(Throwable)so a throw from the caller-suppliedonCapture/onError/onDismisscallbacks can't crash the process.withTimeout(10_000L)sites documenting a known residual risk: Kotlin cancellation is cooperative and cannot preempt a truly HAL-wedgedtakePicture()or a synchronousBitmapFactorydecode mid-call.ExifOrientationFixerTest) locking in the EXIF-fix dispatcher hop, with an honest KDoc about what it does and doesn't cover.Test plan
./gradlew :kmp:compileDebugKotlinAndroid/:kmp:compileTestKotlinJvm— both clean./gradlew :kmp:testDebugUnitTest --tests dev.stapler.stelekit.platform.sensor.ExifOrientationFixerTest— 12/12 passed./gradlew :kmp:testDebugUnitTest --tests dev.stapler.stelekit.platform.sensor.AndroidCameraFrameSourceTest— 3/3 passed (no regression to the concurrent QR-scan camera session)./gradlew :kmp:jvmTest --tests dev.stapler.stelekit.ui.CaptureAndImportTest— 10/10 passed./gradlew :kmp:jvmTest --tests dev.stapler.stelekit.platform.sensor.MotionSensorProviderTest— 10/10 passed./gradlew :kmp:detekt— no violationsKnown residual gaps (not addressed here, called out for visibility)
capturePhoto()/takePhotoAndProcess()call sites (CameraX is not cheaply fakeable under Robolectric) — documented as a known residual risk in this task's validation plan.AndroidCameraProvider.capturePhoto()still has no reachable production caller (pre-existing, tracked as a follow-up pipeline-unification ticket).Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
https://claude.ai/code/session_01VrCZ7tzBPrWhhj7oJXCXKX