fix: prove the FGS video's scan start by notification, not by the service record
The script could report "scan started" on runs where the app was being killed, which sent me
chasing a "mid-scan death" that never happened. The mistake was the signal: dumpsys activity
services lists a service *record* from the moment startForegroundService() is called, so it says
nothing about whether startForeground() was reached - and that gap is exactly what the platform's
deadline measures. logcat's events buffer shows what really happened on every attempt:
am_crash: [..., android.app.ForegroundServiceDidNotStartInTimeException,
Context.startForegroundService() did not then call
Service.startForeground(): .../ScanService]
am_proc_died: [...]
No am_kill, no low-memory record: not memory pressure, and not a crash in the app's own code. The
script now waits for the app's notification, which only exists once startForeground() has been
called, and prints that am_crash line when the service never starts, so a failure is never
mistaken for a success again.
Also in the script:
- let the emulator settle before the run (first-boot work competes for the same CPU) and give the
process time to load its classes before asking it to start a service inside the budget, both
overridable as SETTLE_SECONDS and WARM_SECONDS,
- give up with a non-zero exit and an explicit "record this on a real device" message rather than
carrying on to record a video whose scan never started,
- read the events buffer rather than the main one when reporting the reason.
Verdict for the docs, corrected: this capture cannot be done on a KVM-less emulator, and the
previous "second failure mode" was this one misread. The last crash landed on a process that had
been alive and idle for over a minute, so no amount of warming fixes it - while on the phone the
same build scans 5,363 photos in 20 s with the notification up the whole time. The take belongs on
the phone; the script is the checklist for what it has to show.
This commit is contained in:
@@ -289,21 +289,31 @@ action except the items marked **[code]**.
|
|||||||
|
|
||||||
Keep it under two minutes and don't cut mid-way; a reviewer has to be able to follow
|
Keep it under two minutes and don't cut mid-way; a reviewer has to be able to follow
|
||||||
the trigger and see the service doing its job after the app leaves the screen.
|
the trigger and see the service doing its job after the app leaves the screen.
|
||||||
⚠️ **Record it on a real device.** The capture proved unreliable on this machine's
|
⚠️ **Record it on a real device — it cannot be done on a KVM-less emulator.** Every
|
||||||
software-emulated AVD (no KVM), and both failures are platform-level rather than
|
attempt to start a scan on this machine's software-emulated AVD ended the same way, and
|
||||||
app-level:
|
logcat's events buffer names the cause each time:
|
||||||
|
|
||||||
- A **cold** app can miss Android's 5-second `startForeground` deadline and get killed
|
```
|
||||||
(`ForegroundServiceDidNotStartInTimeException` → *Force finishing activity*), so the
|
am_crash: [..., android.app.ForegroundServiceDidNotStartInTimeException,
|
||||||
launcher appears where the scan should have started. The script's warm-up phase gets a
|
Context.startForegroundService() did not then call
|
||||||
process past that — a process that has already started the service once succeeds — but
|
Service.startForeground(): .../ScanService]
|
||||||
- the app also died **mid-scan** on the emulator during testing, and the launcher was on
|
am_proc_died: [...]
|
||||||
screen afterwards with no exception from the app in logcat at all. A warmer process
|
```
|
||||||
does not fix that one.
|
|
||||||
|
|
||||||
Neither failure has been seen on the real test device, where the same 22-photo library
|
The platform kills the app and force-finishes the activity, so the launcher appears
|
||||||
scans without incident. So treat `tools/record-fgs-demo.sh` as the checklist for the
|
where the scan should have started. It is not a crash in the app's own code, it is not
|
||||||
journey it records, and run it against the phone.
|
memory pressure (there is no `am_kill` or low-memory record), and it is not something a
|
||||||
|
warm-up can fix: the last one happened on a process that had been alive and idle for over
|
||||||
|
a minute. Software emulation is simply too slow to get from `startForegroundService()` to
|
||||||
|
`startForeground()` inside Android's budget — while on the phone this app scans a
|
||||||
|
5,363-photo library in 20 s with the notification up throughout.
|
||||||
|
|
||||||
|
An earlier version of this note claimed a second, unexplained mid-scan death. That was
|
||||||
|
this same failure misread: the script's old "is a scan running?" check matched the
|
||||||
|
service *record*, which exists from the moment `startForegroundService()` is called, so
|
||||||
|
it reported success on runs that were already dying. `tools/record-fgs-demo.sh` now waits
|
||||||
|
for the app's notification instead, which is the only honest proof the service started,
|
||||||
|
and reports the `am_crash` line when it never does.
|
||||||
|
|
||||||
The declaration is **per type**: if a new foreground-service type is ever added to
|
The declaration is **per type**: if a new foreground-service type is ever added to
|
||||||
the manifest, it has to be declared here too.
|
the manifest, it has to be declared here too.
|
||||||
|
|||||||
+1
-1
@@ -29,7 +29,7 @@ Legend: ✅ done · 🚧 in progress · ⏳ not started · 💡 candidate (propo
|
|||||||
|---|---|
|
|---|---|
|
||||||
| ~~Large-library scan stall~~ ✅ **fixed — verified on device** | The 5,363-photo library used to freeze at *“74 / 100 · Comparing file signatures…”* (a **blocked syscall**: zero CPU, workers sleeping). `BoundedCall` now abandons a stuck provider call **and** trips a stall breaker after three consecutive timeouts. **Verified on the real test device (Android 12 / MIUI): the scan completes — 20.2 s cold, 11.0 s warm, 15 groups, 6 unreadable — where it used to hang.** Covered by `BoundedCallTest` and `ScanEngineEndToEndTest` |
|
| ~~Large-library scan stall~~ ✅ **fixed — verified on device** | The 5,363-photo library used to freeze at *“74 / 100 · Comparing file signatures…”* (a **blocked syscall**: zero CPU, workers sleeping). `BoundedCall` now abandons a stuck provider call **and** trips a stall breaker after three consecutive timeouts. **Verified on the real test device (Android 12 / MIUI): the scan completes — 20.2 s cold, 11.0 s warm, 15 groups, 6 unreadable — where it used to hang.** Covered by `BoundedCallTest` and `ScanEngineEndToEndTest` |
|
||||||
| ~~Unreleased `BoundedCall` fix~~ ✅ | The provider-call deadline shipped in **1.0.1** (`versionCode` 2) |
|
| ~~Unreleased `BoundedCall` fix~~ ✅ | The provider-call deadline shipped in **1.0.1** (`versionCode` 2) |
|
||||||
| **Play Console: foreground-service declaration** | Declaring the `dataSync` type (App content › Foreground service permissions) is release-blocking. **[code]** the manifest already declares it, and [PLAY_RELEASE_CHECKLIST](PLAY_RELEASE_CHECKLIST.md) step 5 now carries all four things the form asks for: the description, the user impact if the scan is deferred or interrupted, the use case (**Local processing: Other** — the only `dataSync` option that fits a local, user-initiated scan) and the **video link** Play requires. `tools/record-fgs-demo.sh` records that video — empty state → *Start scan* → Home → the notification in the shade while the scan continues → results — into the git-ignored `store-assets/fgs-demo/`; hosting it and pasting the link is still a manual step. The take itself belongs on a real device: on this machine's KVM-less AVD the app missed Android's 5-second `startForeground` deadline when cold (`ForegroundServiceDidNotStartInTimeException` → *Force finishing activity*) and died mid-scan once with no exception of its own in logcat, neither of which the script's warm-up phase fully removes, and neither of which reproduces on the phone |
|
| **Play Console: foreground-service declaration** | Declaring the `dataSync` type (App content › Foreground service permissions) is release-blocking. **[code]** the manifest already declares it, and [PLAY_RELEASE_CHECKLIST](PLAY_RELEASE_CHECKLIST.md) step 5 now carries all four things the form asks for: the description, the user impact if the scan is deferred or interrupted, the use case (**Local processing: Other** — the only `dataSync` option that fits a local, user-initiated scan) and the **video link** Play requires. `tools/record-fgs-demo.sh` records that video — empty state → *Start scan* → Home → the notification in the shade while the scan continues → results — into the git-ignored `store-assets/fgs-demo/`; hosting it and pasting the link is still a manual step. The take itself belongs on a real device: on this machine's KVM-less AVD every scan start ended in `am_crash … ForegroundServiceDidNotStartInTimeException` → *Force finishing activity*, with no `am_kill` or low-memory record, i.e. software emulation is too slow to reach `startForeground()` inside Android's budget — and that, not a mysterious "mid-scan death", is what an earlier note here was misreading (the script's old check matched the service *record*, which exists before `startForeground()` is called, so it reported success on runs that were already dying). The evidence that this is the emulator and not the app is the phone's own history: 5,363 photos scanned in 20 s with the notification up throughout |
|
||||||
| **Remaining Play paperwork** | Console actions only, no code: the **first upload + Play App Signing enrolment**, the **hosted privacy policy URL** (the working copy is now published to Gitea as [`fishfacegit/foto-duplicate-finder`](https://gitea.softwareapp-hb.de/fishfacegit/foto-duplicate-finder), so the policy renders at a public, login-free URL and only the Console paste is left), **Data safety**, the **Photo/video permissions** declaration, the **content rating / audience / app access / ads / export** questions, and the **listing copy + the three screenshot sets** (phone, 7-inch and 10-inch, all captured), plus the **paid-app setup**: a payments profile with tax rates, the €2.99 price and the 14-day €1.49 launch sale on *Products > App pricing* — the price must be set before the first upload, because free → paid is not possible afterwards (M9). Keystore, `versionCode`, listing copy, graphics, permission audit and the signed `.aab` (`sha256 f8e6a29a…`) are all in place; rollout goes internal → closed → production. Step-by-step: [PLAY_RELEASE_CHECKLIST](PLAY_RELEASE_CHECKLIST.md) |
|
| **Remaining Play paperwork** | Console actions only, no code: the **first upload + Play App Signing enrolment**, the **hosted privacy policy URL** (the working copy is now published to Gitea as [`fishfacegit/foto-duplicate-finder`](https://gitea.softwareapp-hb.de/fishfacegit/foto-duplicate-finder), so the policy renders at a public, login-free URL and only the Console paste is left), **Data safety**, the **Photo/video permissions** declaration, the **content rating / audience / app access / ads / export** questions, and the **listing copy + the three screenshot sets** (phone, 7-inch and 10-inch, all captured), plus the **paid-app setup**: a payments profile with tax rates, the €2.99 price and the 14-day €1.49 launch sale on *Products > App pricing* — the price must be set before the first upload, because free → paid is not possible afterwards (M9). Keystore, `versionCode`, listing copy, graphics, permission audit and the signed `.aab` (`sha256 f8e6a29a…`) are all in place; rollout goes internal → closed → production. Step-by-step: [PLAY_RELEASE_CHECKLIST](PLAY_RELEASE_CHECKLIST.md) |
|
||||||
| ~~Store screenshots need one re-shoot~~ ✅ **done — verified on device** | The old captures showed the *Photo access needed* banner (an app-op is not a grant) and `03-results.png` showed the developer's own library. After a real grant the set was re-shot over the generated demo library: **01-empty** (no tap needed — results and selection live in `ScannerViewModel`, so force-stop + relaunch returns to the empty state with the grant still in place), **02-scanning** (`5353 / 5385`), **03-results** (**17 groups, 54 of 5385 photos, 83.5 MB reclaimable**; the headline group's 43.1 MB out-ranks the device's own 39.8 MB, so no real photo is in frame) and **04-options**. Captures are taken by watching for UI **text** changes, because MIUI refuses synthetic taps (`INJECT_EVENTS`). The demo library is deleted from the test device afterwards — MediaStore **5,386 → 5,364** rows, and the 22 stale rows it leaves in the hash index are pruned by `PhotoIndexStore.pruneTo` on the next scan. Deliberately not captured: the staged state and the system confirmation dialog, because staging clears the demo group and would put real photos back in frame. Details: [STORE_LISTING](STORE_LISTING.md) |
|
| ~~Store screenshots need one re-shoot~~ ✅ **done — verified on device** | The old captures showed the *Photo access needed* banner (an app-op is not a grant) and `03-results.png` showed the developer's own library. After a real grant the set was re-shot over the generated demo library: **01-empty** (no tap needed — results and selection live in `ScannerViewModel`, so force-stop + relaunch returns to the empty state with the grant still in place), **02-scanning** (`5353 / 5385`), **03-results** (**17 groups, 54 of 5385 photos, 83.5 MB reclaimable**; the headline group's 43.1 MB out-ranks the device's own 39.8 MB, so no real photo is in frame) and **04-options**. Captures are taken by watching for UI **text** changes, because MIUI refuses synthetic taps (`INJECT_EVENTS`). The demo library is deleted from the test device afterwards — MediaStore **5,386 → 5,364** rows, and the 22 stale rows it leaves in the hash index are pruned by `PhotoIndexStore.pruneTo` on the next scan. Deliberately not captured: the staged state and the system confirmation dialog, because staging clears the demo group and would put real photos back in frame. Details: [STORE_LISTING](STORE_LISTING.md) |
|
||||||
| ~~Tablet store screenshots outstanding~~ ✅ **done — captured on AVDs** | Play has a **7-inch tablet** and a **10-inch tablet** screenshot slot of its own (asking for **≥ 4** screenshots on large screens). Both are now captured on AOSP Android 12 AVDs — `Nexus 7 2013` (1200×1920, `sw600dp`) and `medium_tablet` (1600×2560, `sw800dp`) — because this machine exposes no `/dev/kvm`: they ran in **pure software emulation** (`-accel off -no-window -gpu swiftshader_indirect`), ≈3 min to boot, with `hide_error_dialogs` on so SystemUI's ANR dialog could not sit in frame. Four screens per tablet over the same generated demo library: empty, scanning (20 / 22), results (**2 groups · 16 of 22 photos · 43.7 MB reclaimable**, 39.7 s / 53.1 s) and Options. `tools/make-store-screenshots.sh` is form-factor aware (`FORM_FACTOR=phone\|tablet-7in\|tablet-10in`) and the tablet form factors crop **0 px** of chrome, because a tablet frame is already inside Play's 2:1 cap. Recipe: [STORE_LISTING](STORE_LISTING.md) |
|
| ~~Tablet store screenshots outstanding~~ ✅ **done — captured on AVDs** | Play has a **7-inch tablet** and a **10-inch tablet** screenshot slot of its own (asking for **≥ 4** screenshots on large screens). Both are now captured on AOSP Android 12 AVDs — `Nexus 7 2013` (1200×1920, `sw600dp`) and `medium_tablet` (1600×2560, `sw800dp`) — because this machine exposes no `/dev/kvm`: they ran in **pure software emulation** (`-accel off -no-window -gpu swiftshader_indirect`), ≈3 min to boot, with `hide_error_dialogs` on so SystemUI's ANR dialog could not sit in frame. Four screens per tablet over the same generated demo library: empty, scanning (20 / 22), results (**2 groups · 16 of 22 photos · 43.7 MB reclaimable**, 39.7 s / 53.1 s) and Options. `tools/make-store-screenshots.sh` is form-factor aware (`FORM_FACTOR=phone\|tablet-7in\|tablet-10in`) and the tablet form factors crop **0 px** of chrome, because a tablet frame is already inside Play's 2:1 cap. Recipe: [STORE_LISTING](STORE_LISTING.md) |
|
||||||
|
|||||||
+48
-20
@@ -112,13 +112,22 @@ wait_for_exact() {
|
|||||||
return 1
|
return 1
|
||||||
}
|
}
|
||||||
|
|
||||||
scan_running() { "${adb[@]}" shell dumpsys activity services 2>/dev/null | grep -q -i ScanService; }
|
|
||||||
|
|
||||||
notifications_from_app() {
|
notifications_from_app() {
|
||||||
"${adb[@]}" shell dumpsys notification --noredact 2>/dev/null \
|
"${adb[@]}" shell dumpsys notification --noredact 2>/dev/null \
|
||||||
| grep -c 'NotificationRecord(.*fotodup' || true
|
| grep -c 'NotificationRecord(.*fotodup' || true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# The signal that actually matters. A service *record* appears as soon as startForegroundService() is
|
||||||
|
# called, so it does not prove that startForeground() succeeded - and that five-second gap is exactly
|
||||||
|
# what kills this app on a slow emulator. The first version of this script used the record as its
|
||||||
|
# "scan started" evidence and therefore reported success on runs where the app was being killed
|
||||||
|
# (logcat's events buffer shows am_crash ForegroundServiceDidNotStartInTimeException moments later).
|
||||||
|
# The app's notification only exists once startForeground() has been called, so wait for that.
|
||||||
|
fgs_started() { [ "$(notifications_from_app)" != "0" ]; }
|
||||||
|
|
||||||
|
# Secondary signal, for logging only.
|
||||||
|
scan_running() { "${adb[@]}" shell dumpsys activity services 2>/dev/null | grep -q -i ScanService; }
|
||||||
|
|
||||||
checkpoint() { # <label> — one PNG per milestone, so the video can be verified without decoding it
|
checkpoint() { # <label> — one PNG per milestone, so the video can be verified without decoding it
|
||||||
"${adb[@]}" exec-out screencap -p > "$out/checkpoint-$1.png" 2>/dev/null || true
|
"${adb[@]}" exec-out screencap -p > "$out/checkpoint-$1.png" 2>/dev/null || true
|
||||||
printf ' checkpoint-%s.png %s\n' "$1" "$(identify -format '%wx%h' "$out/checkpoint-$1.png" 2>/dev/null || echo '?')"
|
printf ' checkpoint-%s.png %s\n' "$1" "$(identify -format '%wx%h' "$out/checkpoint-$1.png" 2>/dev/null || echo '?')"
|
||||||
@@ -165,36 +174,56 @@ sleep 2
|
|||||||
|
|
||||||
# --------------------------------------------------------------------- warm-up
|
# --------------------------------------------------------------------- warm-up
|
||||||
#
|
#
|
||||||
# On a software-emulated device (no /dev/kvm) a *cold* app can miss Android's five-second
|
# The one thing that reliably kills this capture on a software-emulated device (no /dev/kvm) is the
|
||||||
# startForegroundService() deadline. The platform then kills the process with
|
# platform's five-second startForegroundService() deadline: the app is killed with
|
||||||
# ForegroundServiceDidNotStartInTimeException and force-finishes the activity, so the launcher
|
# ForegroundServiceDidNotStartInTimeException and the activity is force-finished, so the launcher
|
||||||
# appears where the scan should have started. That reproduced every single time on a freshly
|
# appears where the scan should have started. logcat's events buffer names it:
|
||||||
# started process and never on one that had already run a scan, so: run a scan, cancel it, and only
|
#
|
||||||
# then record. Cancelling early also leaves the hash index half-built, which keeps the recorded scan
|
# am_crash: [..., android.app.ForegroundServiceDidNotStartInTimeException,
|
||||||
# long enough for the notification to be on screen.
|
# Context.startForegroundService() did not then call Service.startForeground(): ...]
|
||||||
say "Warming the app up (a cold process can miss the FGS deadline - see the note in this script)"
|
# am_proc_died: [...]
|
||||||
|
#
|
||||||
|
# It is a question of CPU, not of the app: the same build starts the service immediately when the
|
||||||
|
# process has been alive for a while (classes loaded, JIT warm) and the emulator is not still doing
|
||||||
|
# first-boot work. So warm the app up first, and only start recording once a real scan has been
|
||||||
|
# confirmed - by the notification, for the reason given at fgs_started().
|
||||||
|
if [ "${SETTLE_SECONDS:-150}" != "0" ]; then
|
||||||
|
say "Letting the emulator settle (first-boot work competes for the same CPU)"
|
||||||
|
sleep "${SETTLE_SECONDS:-150}"
|
||||||
|
fi
|
||||||
|
|
||||||
|
say "Warming the app up (a cold process misses the FGS deadline - see the note in this script)"
|
||||||
attempt=0
|
attempt=0
|
||||||
while [ "$attempt" -lt 3 ]; do
|
while [ "$attempt" -lt 4 ]; do
|
||||||
attempt=$(( attempt + 1 ))
|
attempt=$(( attempt + 1 ))
|
||||||
"${adb[@]}" shell am force-stop "$pkg"
|
"${adb[@]}" shell am force-stop "$pkg"
|
||||||
"${adb[@]}" shell am start -n "$activity" >/dev/null
|
"${adb[@]}" shell am start -n "$activity" >/dev/null
|
||||||
|
# Time for the process to load its classes before it is asked to start a service in under five
|
||||||
|
# seconds. This is the difference between the runs that worked and the ones that crashed.
|
||||||
|
sleep "${WARM_SECONDS:-75}"
|
||||||
if ! start="$(wait_for 'Start scan' 120)"; then
|
if ! start="$(wait_for 'Start scan' 120)"; then
|
||||||
echo "error: the empty state never appeared - right APK installed? photo access granted?" >&2
|
echo "error: the empty state never appeared - right APK installed? photo access granted?" >&2
|
||||||
exit 3
|
exit 3
|
||||||
fi
|
fi
|
||||||
"${adb[@]}" shell input tap $start
|
"${adb[@]}" shell input tap $start
|
||||||
|
started=no
|
||||||
# dumpsys only while the service starts: every `uiautomator dump` spawns a JVM on the device and
|
# dumpsys only while the service starts: every `uiautomator dump` spawns a JVM on the device and
|
||||||
# competes for exactly the CPU that the five-second deadline is measured against.
|
# competes for exactly the CPU that the five-second deadline is measured against.
|
||||||
for _ in $(seq 1 10); do
|
for _ in $(seq 1 12); do
|
||||||
if scan_running; then break; fi
|
if fgs_started; then started=yes; break; fi
|
||||||
sleep 3
|
sleep 3
|
||||||
done
|
done
|
||||||
if scan_running; then
|
if [ "$started" = yes ]; then
|
||||||
printf ' attempt %s: scan started - this process has now met the deadline once\n' "$attempt"
|
printf ' attempt %s: the service started (notification posted)\n' "$attempt"
|
||||||
break
|
break
|
||||||
fi
|
fi
|
||||||
printf ' attempt %s: the service never started\n' "$attempt"
|
printf ' attempt %s: the service never started\n' "$attempt"
|
||||||
|
"${adb[@]}" logcat -b events -d -t 400 2>/dev/null | grep -m1 'am_crash' || true
|
||||||
done
|
done
|
||||||
|
if [ "$started" != yes ]; then
|
||||||
|
echo "error: no scan could be started on this emulator - record the video on a real device" >&2
|
||||||
|
exit 4
|
||||||
|
fi
|
||||||
|
|
||||||
# Let the warm-up scan run to *completion* instead of cancelling it: cancelling does not lead back to
|
# Let the warm-up scan run to *completion* instead of cancelling it: cancelling does not lead back to
|
||||||
# the intro, and restarting the app to get there throws away exactly the warmth that keeps the
|
# the intro, and restarting the app to get there throws away exactly the warmth that keeps the
|
||||||
@@ -225,15 +254,14 @@ fi
|
|||||||
printf ' Scan (top bar) at %s\n' "$scan_button"
|
printf ' Scan (top bar) at %s\n' "$scan_button"
|
||||||
"${adb[@]}" shell input tap $scan_button
|
"${adb[@]}" shell input tap $scan_button
|
||||||
sleep 8 # deliberately dump-free: this is the deadline window
|
sleep 8 # deliberately dump-free: this is the deadline window
|
||||||
if ! scan_running; then
|
if ! fgs_started; then
|
||||||
printf ' the service did not start; the platform said:\n'
|
printf ' the service did not start; the events buffer said:\n'
|
||||||
"${adb[@]}" logcat -d -t 300 2>/dev/null \
|
"${adb[@]}" logcat -b events -d -t 400 2>/dev/null | grep -m1 'am_crash' || true
|
||||||
| grep -m1 -E 'ForegroundServiceDidNotStartInTimeException' || true
|
|
||||||
echo "warn: no scan running - the video will not show the service" >&2
|
echo "warn: no scan running - the video will not show the service" >&2
|
||||||
else
|
else
|
||||||
checkpoint 1-scanning
|
checkpoint 1-scanning
|
||||||
|
printf ' the service started (notification posted) on a process warmed by attempt %s\n' "$attempt"
|
||||||
fi
|
fi
|
||||||
printf ' scan running: %s\n' "$(scan_running && echo yes || echo no)"
|
|
||||||
|
|
||||||
# Away from the app: from here on the service is what keeps the scan alive.
|
# Away from the app: from here on the service is what keeps the scan alive.
|
||||||
sleep 3
|
sleep 3
|
||||||
|
|||||||
Reference in New Issue
Block a user