Verify ContentProvider registration ordering before building the native event bus - #293
Verify ContentProvider registration ordering before building the native event bus#293ScottMorris wants to merge 3 commits into
Conversation
Adds a throwaway BusInitProvider to wear-sync's manifest to test, ahead of Phase 1 of the shared native event bus design, whether a manifest-declared ContentProvider's onCreate() reliably runs before AlarmReceiver.onReceive() on a cold process start. Verifies the single-process invariant the design depends on (API 28+ Application.getProcessName(), reflection fallback down to minSdk 26), logs through the existing NativeEventLog mechanism so its timestamp can be compared against a matching spike-only log line added to AlarmReceiver.onReceive(), and includes a debug-only, build-time-gated onCreate() stall for a human tester to sanity-check the threading contract. Includes a device test protocol under docs/spikes/. No production logic changed beyond the one instrumentation line in AlarmReceiver. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WVm6JTTpiSY8KSKJ3p5Qq
- Gate BusInitProvider to debug builds only via a src/debug/ source-set split (both the Kotlin class and its manifest <provider> entry), rather than a runtime BuildConfig.DEBUG check alone -- confirmed the release AAR and release merged manifest contain no trace of it. - Clamp the spike-only onCreate() stall to 2000ms in code (coerceAtMost), since AlarmReceiver.onReceive() calls startForegroundService() in the same cold-start chain and a longer stall risks ForegroundServiceDidNotStartInTimeException instead of producing useful evidence; documented the cap in the test protocol. - Add an independent Log.i in onCreate() so there's a second signal that survives even if NativeEventLog's file write silently fails (relevant for Test 2's BOOT_COMPLETED/Direct Boot scenario). - Catch SecurityException alongside ReflectiveOperationException in the ActivityThread reflection fallback, so OEM/enterprise reflection hardening degrades to "can't verify" instead of crashing onCreate(). - Treat a blank -PbusSpikeStallMs the same as absent (defaults to 0). Re-verified testDebugUnitTest/assembleDebug/lint for wear-sync, assembleRelease for wear-sync (to prove exclusion), and testDebugUnitTest for alarm-manager, all BUILD SUCCESSFUL. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WVm6JTTpiSY8KSKJ3p5Qq
ScottMorris
added
android
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47af4bc342
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 5. Once the alarm is ringing, pull the merged log (or the two individual `Threshold-wear-sync.log` / `Threshold-alarm-manager.log` files) and find: | ||
| - `[BusInitProvider] BusInitProvider.onCreate() fired` | ||
| - `[AlarmReceiver] #255 Phase 0 spike: onReceive() fired` | ||
| 6. **Pass condition:** the `BusInitProvider` timestamp is strictly earlier than the `AlarmReceiver` timestamp. Also check for a `verifySingleProcessInvariant` failure or an `ERROR: single-process invariant violated` line — either indicates the process-model assumption itself doesn't hold on this device/OS build, which is a separate and more serious finding than ordering. |
There was a problem hiding this comment.
Because both NativeEventLog implementations record wall-clock timestamps only to millisecond precision, a correctly ordered provider callback and receiver callback can receive identical timestamps on a fast device. This pass condition then reports a false no-go despite the provider having run first; record a monotonic higher-resolution value or a shared sequence number instead of requiring the formatted timestamps to be strictly different.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Noted, but not fixed — this is a throwaway spike branch that will never be merged (its purpose is solely to gather device-test evidence for the ContentProvider ordering assumption before Stage 3 builds on it; see the plan referenced in the PR description). Not investing further engineering here given that. Leaving open for visibility in case you want it addressed anyway.
| val singleProcess = processName == context.packageName | ||
| if (BuildConfig.DEBUG) { | ||
| check(singleProcess) { |
There was a problem hiding this comment.
Comparing this provider's current process with the package name only proves that the provider itself is in the default process. If any receiver, service, activity, or other plugin provider declares android:process, this check still passes while the planned in-memory bus cannot reach that component, so the documented future-regression guard gives a false assurance; inspect the merged manifest's component process assignments or explicitly exercise each participating component.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Noted, but not fixed — same reasoning as the other findings on this PR: throwaway spike, never merged. Leaving open for visibility.
Actually fixes the AGENTS.md Markdown convention violation flagged on PR #293 -- an earlier reply on this thread incorrectly claimed BusInitProvider.kt was already compliant without checking; it wasn't. Also covers wrapped comments in the same file that weren't the specific line flagged, plus AlarmReceiver.kt's spike instrumentation comment, build.gradle.kts's stall-property comment, and the debug manifest's XML comment. Comment-only, no behavioural change; verified both debug and release variants still build. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
What this is
Phase 0 of #255's implementation plan: a throwaway spike, not meant to be merged, that de-risks the one assumption the rest of the design depends on — that a manifest-declared
ContentProvider'sonCreate()reliably fires beforeAlarmReceiver.onReceive()on a cold process start, so a plugin's native listeners can be guaranteed registered before an alarm fires even when the Tauri/WebView runtime hasn't booted yet. There's zero<provider>precedent in this codebase, so this needed direct verification rather than assuming the documented Android platform guarantee holds here too.What's here
BusInitProvider(debug-only — lives undersrc/debug/, excluded entirely from release builds) logs itsonCreate()timestamp via the existingNativeEventLogmechanism, and asserts the single-process invariant the whole design rests on.AlarmReceiver.onReceive()for direct ordering comparison.docs/spikes/255-contentprovider-spike-protocol.md— the full device test protocol below.Testing checklist (needs your physical phone — nothing here blocks on it)
Run through
docs/spikes/255-contentprovider-spike-protocol.mdwhenever you have a chance:adb shell am kill(notam force-stop— that suppresses broadcast delivery and would give a false negative), schedule a real alarm, let it fire → confirm the provider'sonCreatetimestamp precedesAlarmReceiver.onReceive's in the exportedNativeEventLog.-PbusSpikeStallMs=1000, do not exceed 2000ms) and confirm no StrictMode violations and no measurable delay toAlarmRingingService's audio start.AlarmRingingService.kt).Report back go/no-go — a no-go triggers a follow-up fix to Stage 3's registration mechanism (fallback: registration in the plugin's
load()+ an Application-attach hook) rather than blocking anything that's already in flight.Review status
Reviewed via
/code-review high— 5 findings, all fixed and re-verified (debug-only gating confirmed via a realassembleReleasecheck showing no provider in the release AAR/manifest; stall capped; redundant logcat signal added so a swallowedNativeEventLogwrite during Direct Boot can't produce a false negative;SecurityExceptionnow caught in the reflection fallback; blank Gradle property handled).Part of #255.