fix(android): prevent camera capture dialog from hanging forever#256
Open
tstapler wants to merge 4 commits into
Open
fix(android): prevent camera capture dialog from hanging forever#256tstapler wants to merge 4 commits into
tstapler wants to merge 4 commits into
Conversation
Four stacked root causes made the capture button spin indefinitely: an untimed sensorDataFlow.firstOrNull() that never emits because startSensing()/stopSensing() were never wired into the dialog/capture lifecycle, synchronous EXIF orientation-fix work running on the caller's dispatcher after the shutter timeout had already elapsed, an unguarded bindToLifecycle() call that could throw uncaught on the camera executor, and no cancel path once isCapturing was true. - Extract MotionSensorProvider.snapshotSensorData() (bounded by withTimeoutOrNull) and share it between CameraViewfinderDialog.android.kt and AndroidCameraProvider.capturePhoto() so the bound can't drift between the two duplicated capture paths. - Wire startSensing()/stopSensing() into the dialog's DisposableEffect and around AndroidCameraProvider.capturePhoto(). - Move ExifOrientationFixer work onto PlatformDispatcher.IO and widen the 10s timeout to cover the whole pipeline (shutter + EXIF fix), not just takePicture(). - Catch Throwable (not Exception) in ExifOrientationFixer and both capture call sites so an OOM on a large frame surfaces as a failure instead of killing the process. - Guard bindToLifecycle() with try/catch -> onError instead of letting it throw uncaught in a bare Runnable listener. - Hold the capture Job so Cancel/dismiss can cancel an in-flight capture instead of being disabled while isCapturing. - Timeout-guard ImageImportService.import() in App.kt's CapturePreviewDialog.onSave so a stalled save can't leave the importing spinner stuck forever. Verified on a booted emulator: a broken camera binding (0 available cameras) now fails fast with no hang or crash instead of wedging the UI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01THQr5q7RczysdVGRGijyXH
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01THQr5q7RczysdVGRGijyXH
…camera-capture-hang # Conflicts: # .backlog-context.md
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.4%` byte[]_[k] `7.6%` java.util.LinkedHashMap$Entry_[k] `7.2%` java.lang.String_[k] `6.4%` int[]_[k] `3.6%` java.lang.StringBuilder_[k]Top CPU hotspots (this PR)`97.9%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0.6%` /tmp/sqlite-3.51.3.0-b1d97c27-73c4-44c0-9a22-d013a389edd9-libsqlitejdbc.so `0.4%` fsync `0.2%` __libc_pwrite `0.1%` pthread_cond_signal |
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.
|
Four-agent parallel review of PR #256 surfaced two real correctness issues and a coverage gap; addressed all three: - Removed startSensing()/stopSensing() from AndroidCameraProvider. capturePhoto(): SensorModule.motionSensorProvider is a shared, non-reference-counted singleton, and this method has no reachable production caller today (see App.kt's unused executeCaptureAndImport). Wiring start/stop here would race an already-open CameraViewfinderDialog's own sensing session — one caller's stopSensing() could kill sensing the other still needs. snapshotSensorData() still applies the actual hang-prevention fix identically to both call sites. - Guarded CameraViewfinderDialog's DisposableEffect startSensing() call with try/catch -> onError: an uncaught throw here previously bypassed the Throwable-hardening this same PR added elsewhere. - Stopped re-wrapping the caught Throwable in takePhotoAndProcess's catch-all — Result.failure(e) instead of Result.failure(Exception(e.message)), so an OOM's real type/stack/cause chain survives instead of being discarded. - Extracted withImportTimeout() so the App.kt post-capture import-timeout logic (previously untested despite being pure suspend/state logic) has a direct regression test proving a stalled import returns null instead of hanging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01THQr5q7RczysdVGRGijyXH
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
1 participant
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
Tapping capture in the camera dialog could hang forever, forcing users to
force-kill the app. Four stacked root causes:
sensorDataFlow.firstOrNull()was untimed and never emitted becausestartSensing()/stopSensing()were never wired into the capturelifecycle in either duplicated implementation.
ExifOrientationFixer's synchronous full-res bitmap decode/rotate/encoderan on the calling dispatcher with no timeout, after the existing 10s
withTimeouthad already elapsed.bindToLifecycle()inside a bareRunnablelistener could throw andwedge the dialog, uncaught.
isCapturing == true— Cancel/dismiss wereboth gated on it.
What Changed
MotionSensorProvider.snapshotSensorData()helper(bounded by
withTimeoutOrNull) used identically by bothCameraViewfinderDialog.android.ktandAndroidCameraProvider.capturePhoto(), so the bound can't drift betweenthe two duplicated capture paths.
startSensing()/stopSensing()into the dialog'sDisposableEffectand aroundAndroidCameraProvider.capturePhoto().ExifOrientationFixerwork ontoPlatformDispatcher.IOand widenedthe 10s timeout to cover the whole pipeline (shutter + EXIF fix), not
just
takePicture().Throwable, notException, inExifOrientationFixerand bothcapture call sites, so an
OutOfMemoryErroron a large frame surfaces asa failure instead of killing the process.
bindToLifecycle()with try/catch →onErrorinstead of lettingit throw uncaught.
Jobso Cancel/dismiss can cancel an in-flight captureinstead of being disabled while capturing.
ImageImportService.import(...)inApp.kt'sCapturePreviewDialog.onSaveso a stalled save can't leave the importingspinner stuck forever.
Test plan
./gradlew :kmp:compileDebugKotlinAndroid— builds clean./gradlew :kmp:detekt— clean (caught and fixed a realDisposableEffect/onErrorstaleness bug along the way)./gradlew :kmp:jvmTest— 3894 tests, only 4 pre-existingGraphManagerDatabaseLifecycleTestfailures unrelated to this change(confirmed identical on unmodified
mainviagit stash)./gradlew :kmp:testDebugUnitTest --tests "dev.stapler.stelekit.platform.sensor.*"— all green./gradlew :kmp:testDebugUnitTest --tests "*Qr*" --tests "*Transfer*"— all green, no regression to concurrent QR camera useMotionSensorProviderTest.snapshotSensorData_shouldReturnNull_When_FlowNeverEmits/_shouldReturnLatestReading_When_FlowHasEmittedbindToLifecyclefailure (0 available cameras) now fails fast with no crash/hang instead of wedging the UI🤖 Generated with Claude Code
https://claude.ai/code/session_01THQr5q7RczysdVGRGijyXH