fix(android): buffer calls made before setup and move its blocking work off the caller's thread - #774
Draft
posthog[bot] wants to merge 10 commits into
Draft
Conversation
…rk off the caller's thread capture, screen, identify and register calls made before setup() were dropped with a log line, so app open, the first screen view and deep link attribution raced init and were lost. They are now held in a bounded in-memory buffer and replayed, in order and with the time they were made, once the SDK is enabled. On the Android side, the PackageManager binder call and the release identifier asset read move to a background thread, the app install integration does its PackageManager lookup and preferences work there too, and the storage paths no longer create directories on the caller's thread. Setting the SDK up off the main thread is now safe, because nothing captured meanwhile is lost. Generated-By: PostHog Desktop Task-Id: d6d48be8-4b7a-4a86-8778-abb07420c335
Contributor
Author
🦔 PostHog Review reviewed this pull requestFound 2 must fix, 8 should fix, 1 consider. Published 11 findings (view the review). Resolved comments: 1 already settled |
Contributor
posthog-android Compliance ReportDate: 2026-09-10 13:44:49 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
Author
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Comment on lines
+407
to
+419
| calls.forEach { call -> | ||
| try { | ||
| when (call) { | ||
| is PostHogPreSetupCall.Capture -> | ||
| capture( | ||
| call.event, | ||
| distinctId = call.distinctId, | ||
| properties = call.properties, | ||
| userProperties = call.userProperties, | ||
| userPropertiesSetOnce = call.userPropertiesSetOnce, | ||
| groups = call.groups, | ||
| timestamp = call.timestamp, | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Replay restores PackageManager work to the setup thread
Why we think it's a valid issue
- Checked: Whether
setup()runs on the caller's thread, what the first replayedcapture()touches on Android, what each later replayed call costs on that thread, and whether the PR's own guarantee test covers the buffered case. - Found:
setup()runs its whole body synchronously undersynchronized(setupLock)on the caller's thread (posthog/src/main/java/com/posthog/PostHog.kt:154), andreplayPreSetupCalls()is its last statement (PostHog.kt:387). The replay loop callscapture()directly (PostHog.kt:407-419), so every buffered call executes on the setup thread. - Found:
capture()builds properties on the calling thread.buildPropertiesreadsconfig?.context?.getStaticContext()andgetDynamicContext()atPostHog.kt:704-712. On Android,getStaticContext()returns theby lazycacheStaticContext, whose initializer callsgetPackageInfo(context, config)andcontext.applicationInfo.loadLabel(context.packageManager)(posthog-android/src/main/java/com/posthog/android/internal/PostHogAndroidContext.kt:30-58). The first replayed capture therefore performs both PackageManager operations on the setup thread. - Found: The PR's own guarantee test does not cover this.
setup does not touch the PackageManager on the calling thread(posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt:163-180) buffers no pre-setup call and assertsassertFalse(threads.contains(Thread.currentThread())). Its stub records the thread of eachapp.packageManagerread, so the same test with one bufferedcapture()beforesetup()would record the caller's thread and fail. The stated guarantee holds only while the buffer is empty — the case the feature does not exist for. - Found: Each later replayed capture also costs binder work on that thread, not only the first.
getDynamicContext()callsnetworkPropertiesProvider()andcontext.telephonyManager()?.networkOperatorNameper event (PostHogAndroidContext.kt:65-88). - Found: The cost claim for the rest of the loop is overstated. Per-event disk work is dispatched, not inline:
PostHogQueue.addhands the record toexecutor.executeSafely(posthog/src/main/java/com/posthog/internal/PostHogQueue.kt:137-147). A replayedregisterwrites preferences in memory and commits withedit.apply()(posthog-android/src/main/java/com/posthog/android/internal/PostHogSharedPreferences.kt:171), so it is not a synchronous disk write. The 1000 figure is also the buffer ceiling, not a startup shape:PostHogPreSetupBuffercaps atMAX_SIZE = 1000and its KDoc treats an overflow as "setup() was never reached". - Impact: For a host that keeps
setup()on the main thread and has any pre-setup capture,getPackageInfoandloadLabelreturn toApplication.onCreate(), plus one connectivity and telephony read per buffered capture. StrictMode still flags the setup frame, so the PR's headline claim does not hold for that configuration and its test does not detect it. A cheap remedy already fits the PR's shape: warmgetStaticContext()on the existinginitExecutoralongside the release-identifier work atposthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt:238-245. - Priority: Lowered to
should_fix. In the scenario the PR targets, the host movessetup()to a background thread — the buffer is what makes that safe, and the KDoc atPostHogAndroid.kt:108-114now documents it — so the replay work lands off the main thread and no ANR risk returns. The residual main-thread cost is bounded by the number of buffered calls and is the same per-capture work the SDK already runs on any caller's thread. The suggested primary fix also carries a cost the author should weigh: replaying on a background executor lets calls the host makes right aftersetup()returns overtake the replayed ones, which breaks the oldest-first ordering the replay KDoc promises atPostHog.kt:394-397.
Issue description
replayPreSetupCalls() invokes every buffered call on the thread inside setup(). On Android, the first capture runs PostHogAndroidContext's lazy static-context initializer. That initializer calls getPackageInfo and applicationInfo.loadLabel(context.packageManager). A buffered capture therefore restores PackageManager work to the setup caller. The loop can also run 1,000 preference and property operations there.
Suggested fix
Run replay on an SDK background executor while the buffer remains DRAINING. Alternatively, publish the Android static context from a background result before replay. Add a test that buffers a capture before setup and records every PackageManager access thread.
Prompt to fix with AI (copy-paste)
## Context
@posthog/src/main/java/com/posthog/PostHog.kt#L407-419
<issue_description>
`replayPreSetupCalls()` invokes every buffered call on the thread inside `setup()`. On Android, the first capture runs `PostHogAndroidContext`'s lazy static-context initializer. That initializer calls `getPackageInfo` and `applicationInfo.loadLabel(context.packageManager)`. A buffered capture therefore restores PackageManager work to the setup caller. The loop can also run 1,000 preference and property operations there.
</issue_description>
<issue_validation>
- **Checked:** Whether `setup()` runs on the caller's thread, what the first replayed `capture()` touches on Android, what each later replayed call costs on that thread, and whether the PR's own guarantee test covers the buffered case.
- **Found:** `setup()` runs its whole body synchronously under `synchronized(setupLock)` on the caller's thread (`posthog/src/main/java/com/posthog/PostHog.kt:154`), and `replayPreSetupCalls()` is its last statement (`PostHog.kt:387`). The replay loop calls `capture()` directly (`PostHog.kt:407-419`), so every buffered call executes on the setup thread.
- **Found:** `capture()` builds properties on the calling thread. `buildProperties` reads `config?.context?.getStaticContext()` and `getDynamicContext()` at `PostHog.kt:704-712`. On Android, `getStaticContext()` returns the `by lazy` `cacheStaticContext`, whose initializer calls `getPackageInfo(context, config)` and `context.applicationInfo.loadLabel(context.packageManager)` (`posthog-android/src/main/java/com/posthog/android/internal/PostHogAndroidContext.kt:30-58`). The first replayed capture therefore performs both PackageManager operations on the setup thread.
- **Found:** The PR's own guarantee test does not cover this. `setup does not touch the PackageManager on the calling thread` (`posthog-android/src/test/java/com/posthog/android/PostHogAndroidTest.kt:163-180`) buffers no pre-setup call and asserts `assertFalse(threads.contains(Thread.currentThread()))`. Its stub records the thread of each `app.packageManager` read, so the same test with one buffered `capture()` before `setup()` would record the caller's thread and fail. The stated guarantee holds only while the buffer is empty — the case the feature does not exist for.
- **Found:** Each later replayed capture also costs binder work on that thread, not only the first. `getDynamicContext()` calls `networkPropertiesProvider()` and `context.telephonyManager()?.networkOperatorName` per event (`PostHogAndroidContext.kt:65-88`).
- **Found:** The cost claim for the rest of the loop is overstated. Per-event disk work is dispatched, not inline: `PostHogQueue.add` hands the record to `executor.executeSafely` (`posthog/src/main/java/com/posthog/internal/PostHogQueue.kt:137-147`). A replayed `register` writes preferences in memory and commits with `edit.apply()` (`posthog-android/src/main/java/com/posthog/android/internal/PostHogSharedPreferences.kt:171`), so it is not a synchronous disk write. The 1000 figure is also the buffer ceiling, not a startup shape: `PostHogPreSetupBuffer` caps at `MAX_SIZE = 1000` and its KDoc treats an overflow as "setup() was never reached".
- **Impact:** For a host that keeps `setup()` on the main thread and has any pre-setup capture, `getPackageInfo` and `loadLabel` return to `Application.onCreate()`, plus one connectivity and telephony read per buffered capture. StrictMode still flags the setup frame, so the PR's headline claim does not hold for that configuration and its test does not detect it. A cheap remedy already fits the PR's shape: warm `getStaticContext()` on the existing `initExecutor` alongside the release-identifier work at `posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt:238-245`.
- **Priority:** Lowered to `should_fix`. In the scenario the PR targets, the host moves `setup()` to a background thread — the buffer is what makes that safe, and the KDoc at `PostHogAndroid.kt:108-114` now documents it — so the replay work lands off the main thread and no ANR risk returns. The residual main-thread cost is bounded by the number of buffered calls and is the same per-capture work the SDK already runs on any caller's thread. The suggested primary fix also carries a cost the author should weigh: replaying on a background executor lets calls the host makes right after `setup()` returns overtake the replayed ones, which breaks the oldest-first ordering the replay KDoc promises at `PostHog.kt:394-397`.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Run replay on an SDK background executor while the buffer remains DRAINING. Alternatively, publish the Android static context from a background result before replay. Add a test that buffers a capture before setup and records every PackageManager access thread.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Escalating this one rather than patching it — the finding holds, but every fix I can see is a decision rather than a mechanical change.
Confirmed as described: setup runs its whole body on the caller's thread and replays the buffer as its last step, so the first replayed capture initialises the lazily-built static app context there — which is where the getPackageInfo binder call and the app-label read live. The PR's test genuinely does not cover this: it buffers nothing before setup, so it only proves the guarantee for an empty buffer, which is the case the feature does not exist for. Your corrections about the rest of the loop match the code too — queue writes are dispatched and register commits asynchronously, so the residual cost is the per-capture context reads, not disk.
Why I did not just warm the static context on the existing init executor:
- It is best-effort, not a guarantee. Whether the background warm-up gets to the lazy initialiser before the replay does is a scheduling race. When it wins, the caller merely blocks on the lock and touches nothing itself; when it loses, the caller does the work. A test extending the existing thread check with a buffered capture would be timing-dependent, so I would be shipping a flaky assertion of a hard guarantee — worse than the gap.
- Making it deterministic costs something real. Either setup waits on the warm-up before replaying (a wait back in
Application.onCreate(), and a deadlock risk since the init executor is single-threaded and shared with the app-install work), or the replay itself moves to a background thread (which gives up the ordered oldest-first replay the buffer was built for, and lets post-setup calls overtake buffered ones). - It calls host code on a new thread. The context provider is a public interface that a host can supply, and the Android one is only constructed when the host has not set their own — the wrapper SDKs being the obvious case. Warming would invoke someone else's implementation on a background thread for the first time.
What a human needs to decide: whether to accept the residual first-capture cost and instead narrow the PR's claim ("setup does not block on the PackageManager; the first captured event still resolves the app context on whichever thread captures it", which is also true before this PR), or to take one of the deterministic options above with its stated cost, or to make the static context resolve asynchronously and let the earliest events ship without app version/name — the same fidelity trade-off already made deliberately for the release identifier. That last one changes what the first events carry, so it belongs with the SDK owners.
No code change committed for this thread.
Comment on lines
+108
to
+114
| * Safe to call from a background thread. `capture`, `identify` and `register` calls the | ||
| * host makes before this returns are held in memory and replayed once the SDK is enabled, | ||
| * so app open, the first screen view and deep link attribution are not lost to the race. | ||
| * The cost of setting up off the main thread is that the SDK's own Activity lifecycle | ||
| * callbacks register later, so an Activity created in the meantime is not observed: call | ||
| * [capturePushNotificationOpened] with the launch intent if you need that path. | ||
| * |
Contributor
Author
There was a problem hiding this comment.
Keep Android UI integration setup on the main thread
Why we think it's a valid issue
- Checked: What the two integrations do on the caller's thread, the thread contract of the Curtains version this module pins, where the install loop runs, and whether the SDK already has a main-thread pattern for an integration install.
- Found: Curtains states the contract in its own class documentation: "All properties defined in this class should be accessed from the main thread" (
Curtains.ktincom.squareup.curtains:curtains:1.2.5, the version pinned atbuildSrc/src/main/java/PosthogBuildConfig.kt:68). Both flagged installs read those properties directly.PostHogTouchActivityIntegration.installiteratesCurtains.rootViewsand doeswindow.touchEventInterceptors += touchInterceptor(posthog-android/src/main/java/com/posthog/android/internal/PostHogTouchActivityIntegration.kt:63-72).PostHogReplayIntegration.installrunsCurtains.rootViews.forEach { addView(view) }(posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:578-580), andaddViewregistersdecorView.onNextDraw(...), callsdecorView.viewTreeObserver?.addOnGlobalLayoutListener(layoutListener), and mutateswindow.touchEventInterceptors(PostHogReplayIntegration.kt:325-364). - Found: Both integrations are added on the caller's thread (
posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt:251-252) and installed from the setup loop atposthog/src/main/java/com/posthog/PostHog.kt:332-334, which also runs on the caller's thread. Nothing between the new KDoc and the Curtains calls hops to the main thread. - Found: The first Curtains access is itself unsafe off the main thread, even when no Activity exists yet.
CurtainsholdsRootViewsSpyin alazy(NONE)field, andRootViewsSpy.install()callsWindowManagerSpy.swapWindowManagerGlobalMViews, which reflectively readsWindowManagerGlobal.mViewsand writes back a delegatingArrayListwith no lock. A window that the main thread adds between that read and that write is lost from the framework's own list.Curtains.rootViewsthen returnsdelegatingViewList.toList(), a copy of a plainArrayListthat the main thread mutates, so an off-thread read can throw or observe a torn state.onRootViewsChangedListenersis aCopyOnWriteArrayList, so adding the listener is the one safe step. - Found: A failure is swallowed and latches the integration as installed. Both installs set the static
integrationInstalledAtomicBooleanandownsInstallationbefore the guarded block (PostHogTouchActivityIntegration.kt:57-62,PostHogReplayIntegration.kt:555-559), the work sits insidetry/catchthat only logs (PostHogTouchActivityIntegration.kt:63-74,PostHogReplayIntegration.kt:314,PostHogReplayIntegration.kt:359-361), and the setup loop catches anything left over (PostHog.kt:356). So no host-visible error appears, and a later reinstall is blocked for the process lifetime. - Found: The precedent the suggestion names is already in the codebase and applies to one integration only.
PostHogLifecycleObserverIntegration.installchecksisMainThread(mainHandler)and postsadd()to the main handler otherwise (posthog-android/src/main/java/com/posthog/android/internal/PostHogLifecycleObserverIntegration.kt:137-143). AMainHandleris constructed in setup and is already handed to the replay integration (PostHogAndroid.kt:250-251), so the mechanism for the same treatment exists. - Found: The new KDoc names one trade-off of background setup, the later registration of Activity lifecycle callbacks (
PostHogAndroid.kt:111-113). It does not mention that the replay and touch installs run UI-thread work, so a host reading "Safe to call from a background thread" gets no signal about this. - Impact: The hazard follows from the diff, because the KDoc is what invites hosts to call
setup()off the main thread. With no Activity yet, the reflectivemViewsswap races the launch Activity's window add. With an Activity present — the late-setup case the buffer KDoc itself names atposthog/src/main/java/com/posthog/internal/PostHogPreSetupBuffer.kt:35-37— view listener registration and window callback mutation happen off the main thread, which can throw on the main thread or leave replay and touch-driven session activity silently half installed and unable to retry.
Issue description
The new documentation says that background setup is safe. However, setup installs PostHogReplayIntegration and PostHogTouchActivityIntegration on the caller's thread. Their install methods inspect Curtains.rootViews and change View listeners and window interceptors. Late background setup can therefore access existing Views off the main thread. These operations can fail or race UI dispatch, which leaves replay and session activity tracking partly installed.
Suggested fix
Move all View and Curtains installation work to MainHandler, as PostHogLifecycleObserverIntegration does. Add cancellation guards for close calls. Test background setup after an Activity exists. Remove the safety statement until these operations use the main thread.
Prompt to fix with AI (copy-paste)
## Context
@posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt#L108-114
<issue_description>
The new documentation says that background setup is safe. However, setup installs `PostHogReplayIntegration` and `PostHogTouchActivityIntegration` on the caller's thread. Their install methods inspect `Curtains.rootViews` and change View listeners and window interceptors. Late background setup can therefore access existing Views off the main thread. These operations can fail or race UI dispatch, which leaves replay and session activity tracking partly installed.
</issue_description>
<issue_validation>
- **Checked:** What the two integrations do on the caller's thread, the thread contract of the Curtains version this module pins, where the install loop runs, and whether the SDK already has a main-thread pattern for an integration install.
- **Found:** Curtains states the contract in its own class documentation: "All properties defined in this class should be accessed from the main thread" (`Curtains.kt` in `com.squareup.curtains:curtains:1.2.5`, the version pinned at `buildSrc/src/main/java/PosthogBuildConfig.kt:68`). Both flagged installs read those properties directly. `PostHogTouchActivityIntegration.install` iterates `Curtains.rootViews` and does `window.touchEventInterceptors += touchInterceptor` (`posthog-android/src/main/java/com/posthog/android/internal/PostHogTouchActivityIntegration.kt:63-72`). `PostHogReplayIntegration.install` runs `Curtains.rootViews.forEach { addView(view) }` (`posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:578-580`), and `addView` registers `decorView.onNextDraw(...)`, calls `decorView.viewTreeObserver?.addOnGlobalLayoutListener(layoutListener)`, and mutates `window.touchEventInterceptors` (`PostHogReplayIntegration.kt:325-364`).
- **Found:** Both integrations are added on the caller's thread (`posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt:251-252`) and installed from the setup loop at `posthog/src/main/java/com/posthog/PostHog.kt:332-334`, which also runs on the caller's thread. Nothing between the new KDoc and the Curtains calls hops to the main thread.
- **Found:** The first Curtains access is itself unsafe off the main thread, even when no Activity exists yet. `Curtains` holds `RootViewsSpy` in a `lazy(NONE)` field, and `RootViewsSpy.install()` calls `WindowManagerSpy.swapWindowManagerGlobalMViews`, which reflectively reads `WindowManagerGlobal.mViews` and writes back a delegating `ArrayList` with no lock. A window that the main thread adds between that read and that write is lost from the framework's own list. `Curtains.rootViews` then returns `delegatingViewList.toList()`, a copy of a plain `ArrayList` that the main thread mutates, so an off-thread read can throw or observe a torn state. `onRootViewsChangedListeners` is a `CopyOnWriteArrayList`, so adding the listener is the one safe step.
- **Found:** A failure is swallowed and latches the integration as installed. Both installs set the static `integrationInstalled` `AtomicBoolean` and `ownsInstallation` before the guarded block (`PostHogTouchActivityIntegration.kt:57-62`, `PostHogReplayIntegration.kt:555-559`), the work sits inside `try`/`catch` that only logs (`PostHogTouchActivityIntegration.kt:63-74`, `PostHogReplayIntegration.kt:314`, `PostHogReplayIntegration.kt:359-361`), and the setup loop catches anything left over (`PostHog.kt:356`). So no host-visible error appears, and a later reinstall is blocked for the process lifetime.
- **Found:** The precedent the suggestion names is already in the codebase and applies to one integration only. `PostHogLifecycleObserverIntegration.install` checks `isMainThread(mainHandler)` and posts `add()` to the main handler otherwise (`posthog-android/src/main/java/com/posthog/android/internal/PostHogLifecycleObserverIntegration.kt:137-143`). A `MainHandler` is constructed in setup and is already handed to the replay integration (`PostHogAndroid.kt:250-251`), so the mechanism for the same treatment exists.
- **Found:** The new KDoc names one trade-off of background setup, the later registration of Activity lifecycle callbacks (`PostHogAndroid.kt:111-113`). It does not mention that the replay and touch installs run UI-thread work, so a host reading "Safe to call from a background thread" gets no signal about this.
- **Impact:** The hazard follows from the diff, because the KDoc is what invites hosts to call `setup()` off the main thread. With no Activity yet, the reflective `mViews` swap races the launch Activity's window add. With an Activity present — the late-setup case the buffer KDoc itself names at `posthog/src/main/java/com/posthog/internal/PostHogPreSetupBuffer.kt:35-37` — view listener registration and window callback mutation happen off the main thread, which can throw on the main thread or leave replay and touch-driven session activity silently half installed and unable to retry.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Move all View and Curtains installation work to `MainHandler`, as `PostHogLifecycleObserverIntegration` does. Add cancellation guards for close calls. Test background setup after an Activity exists. Remove the safety statement until these operations use the main thread.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Escalating — the finding is right, and on checking it the blast radius is wider than the thread says, but the fix is not something I should land unattended.
What I confirmed. The replay and touch integrations are both registered unconditionally, and each one's support check is only an OS-version check, not a "is session replay on" check. So on Android 8+ this applies to every host that takes the new advice and calls setup from a background thread, not just hosts using session replay. The install loop runs inside setup on the caller's thread, and nothing between the new KDoc and the Curtains calls hops to the main thread, so both integrations touch window state off the main thread exactly as described. The latching behaviour holds too, and one part is worse than reported: in the replay integration the root-views loop sits outside the try/catch, so if it throws, the root-view-changed listener is never registered while the integration is already latched as installed — session replay would be silently half-installed for the life of the process, with only a log line.
Why I did not implement the main-thread hop:
- I cannot verify it. This sandbox has no Android SDK, so the
posthog-androidunit tests cannot be run — I could not even prove a change to those files compiles. Shipping an unverified edit to session replay's installation path is not a trade I should make on my own. - It is not mechanical. The touch integration would need a main handler passed in that it does not currently take; both
uninstall()paths touch Curtains the same way, so fixing onlyinstall()is half a fix; and posting the replay registration meansinstall()returns before any view is registered, while the setup loop starts session replay on the very next statement. That ordering interaction with the first captured frame is precisely what needs a real device to confirm. - Neither integration file is part of this PR. Restructuring how session replay installs itself is a bigger change than the one this PR set out to make.
What a human needs to decide, in the order I'd suggest considering them:
- Do the threading fix properly — give both integrations the main-handler treatment the lifecycle observer integration already uses, on install and uninstall, and settle whether the replay start needs to wait for the posted registration. Needs a device or emulator run to confirm the first replay frame is unaffected.
- Or scope the claim instead — keep the behaviour and narrow the KDoc, saying that setup off the main thread is safe for the event path but that the replay and touch integrations register window callbacks on whatever thread calls setup, so hosts using them should stay on the main thread. Cheap and honest, but it noticeably narrows what the PR offers, which is why I am not choosing it for you.
Separately, and independent of which route is taken: it is worth moving the replay root-views loop inside the existing try/catch so a failure there does not skip the listener registration while leaving the integration marked installed.
No code change committed for this thread.
…er events screen() wrote lastScreenName immediately, but buffered events take their $screen_name from lastScreenName at replay time. A capture() made before a pre-setup screen() was therefore stamped with a screen the user had not reached yet, and with several pre-setup screens every buffered event collapsed onto the last title. screen() now buffers a PostHogPreSetupCall.Screen instead of handing the event to capture(), so lastScreenName is only written when the call is replayed and the pre-setup order is preserved. The Screen call carries the time it was made, so the replayed $screen event keeps its own timestamp. An overflowing screen call no longer names later events either, since a dropped call never reaches the replay. Generated-By: PostHog Desktop Task-Id: 706bd50c-6fb5-4968-9b82-3c35c25afae8
The pre-setup buffer keyed off `enabled`, which is false both before the first setup and after close(). A call made after close() was therefore held and replayed by the next setup() - under that setup's config, so with a different API key the event went to another project and a replayed register wrote a super property into its store. It also made a closed SDK silent where it used to log, and retained up to 1000 calls for the rest of the process when close() was a teardown. A `closed` flag now tells the two disabled states apart. It is set with `enabled = false` in close() so no call lands in the teardown gap, and cleared in setup() once setup is committed, so the calls racing the rest of setup are still buffered. A call arriving while closed is dropped with the "Setup isn't called." log it had before the buffer existed. Generated-By: PostHog Desktop Task-Id: 706bd50c-6fb5-4968-9b82-3c35c25afae8
This PR moved the releaseIdentifier resolution onto the init thread, and nothing joins that task, so there was no happens-before edge from the write to the capture threads that read it. captureException() passes config.releaseIdentifier straight into the coercer on the calling thread - for an uncaught exception, the crashing thread - and the coercer omits the frame's map_id when it reads as null or empty, so a stack trace could stay obfuscated even after the task had finished. @volatile gives the write a release edge, which is the convention AGENTS.md states for fields accessed across threads and what optOut, requestHeaders and tracingHeadersList in this same class already use. The annotation only changes the private backing field, so the public API dump is unchanged. The KDoc now states the timing the asynchronous resolution introduces: a value the host assigns after setup can still be replaced by the fallback, so it belongs before setup. Generated-By: PostHog Desktop Task-Id: f0ba55a9-589c-4fd0-95d2-6710760d5f06
The buffer held the caller's maps by reference, so a host that fills one MutableMap, captures, changes a value and captures again got two identical replayed events with the final values - where the enabled path gives two distinct events, because it copies top-level properties into the event map (props.putAll) and copies groups in mergeGroups at call time. A mutation from another thread during the replay could also throw ConcurrentModificationException inside that putAll, and the replay loop catches and logs, so the event was dropped silently. The buffered call entries now copy their map inputs on construction, which is also where the invariant belongs: any future enqueue site gets it. The window a buffered call is exposed for runs until setup() is reached, so this covers userProperties/userPropertiesSetOnce too, even though the enabled path stores those by reference under $set/$set_once until the queue serializes them. Register's value is left alone: it is an Any, not a map, and the enabled path hands the same reference to the preferences store. Generated-By: PostHog Desktop Task-Id: f0ba55a9-589c-4fd0-95d2-6710760d5f06
The drain took a single snapshot, so a caller that had already read `enabled` as false and added after that snapshot left its call in the buffer for good: the SDK is enabled by then, a second setup() is refused, and close() discards the buffer. The replay now drains in a loop until it comes back empty, which covers the whole window the replay itself takes rather than one instant of it. It cannot spin: the calls it replays re-enter the public methods with the SDK enabled, so they are not buffered again. A setup that throws after enabling the SDK also left the buffer stranded, with only a "Setup failed" line to show that the startup calls this buffer exists to save were dropped. Such an SDK still accepts live calls, so the replay now runs from the failure path too when the enable flip already happened. Not addressed here, deliberately: buffered calls still replay after any live call made in the window between the enable flip and the drain, so a buffered register or identify can overwrite a newer live value for the same key. Making that ordering hold needs the buffer-or-live decision and the drain to be mutually exclusive, which is a design change on every capture path. Generated-By: PostHog Desktop Task-Id: f0ba55a9-589c-4fd0-95d2-6710760d5f06
install() now queues the install/update work instead of running it inline inside PostHog.setup(), and uninstall() cannot cancel a queued task. A task that took its turn on the shared init thread after a close still wrote the VERSION and BUILD values - the marker that says the event was already reported - while its own capture was dropped, so that install or update was never reported again on any later launch. If a second setup() had landed instead, the same stale event went out under the new config. The task now checks the ownership flag it already sets in install(), so a queued run after uninstall() does nothing at all: no PackageManager lookup, no marker write, no capture. The flag is @volatile so the init thread sees the uninstall. This leaves an instruction-wide window between the check and the marker write, which would need close() to interleave exactly there; closing that too means holding the integration's monitor across the capture, and the capture path takes the SDK's setup lock, so that trades this for a deadlock. Generated-By: PostHog Desktop Task-Id: f0ba55a9-589c-4fd0-95d2-6710760d5f06
close() returns early when the SDK was never enabled, so the buffer holding calls made before the first setup() survived the close and the next setup() replayed them under a config that close was never part of. Clear the buffer on that path too, before the early return. Generated-By: PostHog Desktop Task-Id: 0930b19f-11c3-4c17-8bc6-42cdbf5c503a
identify() and register() were buffered while the SDK was disabled, but their
inverses returned early, so a pre-setup identify("A") followed by reset()
replayed only the identify and left the SDK associated with — and DISTINCT_ID
holding — the user the host had logged out. A register followed by unregister
restored the property the host removed. Both are worse than dropping the pair,
which is what happened before the buffer existed.
Buffer Reset and Unregister as well, replayed in call order with the rest, so
a sequence and its inverse land the same way they would have after setup.
Generated-By: PostHog Desktop
Task-Id: 0930b19f-11c3-4c17-8bc6-42cdbf5c503a
executeSafely only guards the submission, not the task, so moving this work onto the background executor dropped the containment it had while it ran inline under setup's per-integration try/catch. A host-supplied preferences implementation or logger that throws would reach the worker's uncaught handler, which on Android ends the process. Wrap the task body in its own try/catch that logs, matching what the inline path did. The installation guards are left as they were: the inline path did not release them on failure either. Generated-By: PostHog Desktop Task-Id: 0930b19f-11c3-4c17-8bc6-42cdbf5c503a
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.
💡 Motivation and Context
PostHogAndroid.setup()does its disk and IPC work inline on the caller's thread — the thread our docs point atApplication.onCreate(). StrictMode flags the frame.capture,screen,identifyandregistermade before setup finishes is dropped with a log line. App open, the first screen view and deep link attribution race init and lose.Note
Impact could not be sized from analytics: the project has no
posthog-androidevents, and the SDK does not capture ANRs. The production evidence is the customer's crash reporter, via this inbox report.Changes
1. Pre-setup calls are buffered and replayed, not dropped (
posthog/)PostHogPreSetupBufferholds up to 1000 calls in memory.setup()drains it, oldest first, after the SDK is enabled.screen()lost its own enabled gate: it only caches the title and hands the event tocapture(), which now buffers.setup()was never reached.2. The blocking work moves off the caller's thread (
posthog-android/)PackageManager.getPackageInfo(binder call)getPackageInfo, prefs read + write, captureinAppIncludesgetPackageInfoContext.getPackageName, already in memoryContext.getDir, creates the directoryOnly error tracking reads
releaseIdentifierandinAppIncludes, so nothing in setup waits on them. An exception captured in the first moments carries no release identifier.3. Off-main-thread setup is now documented as safe, on
setup()'s KDoc, with the trade-off stated: our Activity lifecycle callbacks register later, so an Activity created in the meantime is not observed.Important
What is not moved: the SharedPreferences hydration and device-id seeding inside core
PostHog.setup(). The integration install loop registers the Activity lifecycle callbacks, and that has to happen before the launch Activity is created or the first screen view and the deep link are never seen. Making all of setup asynchronous by default would trade an ANR for lost startup events. The buffer is what makes a host-side backgroundsetup()call safe instead.Warning
Buffering
capture/identify/registerbefore init is cross-SDK behaviour. Per this team's convention it should be proposed insdk-specsfirst and ported per platform; that proposal is not part of this PR.💚 How did you test it?
Automated, all green:
PostHogPreSetupTest(5 tests): a capture, a screen, an identify and a register made beforesetup()each reach the server or the store after it; a replayed capture carries the time the call was made, not the setup time; a reservedregisterkey is still rejected rather than buffered.PostHogPreSetupBufferTest(4 tests): drain order, drain empties, overflow keeps the oldest and counts the rest, clear.PostHogAndroidTest.setup does not touch the PackageManager on the calling thread— records the thread of everypackageManageraccess, waits for one to land, asserts it was not the caller's.:posthog:test(all pass) and:posthog-android:testDebugUnitTest(525 tests, all pass), plusspotlessCheckandapiCheck. No public API change.Not tested, and why
No device or emulator run, and no Systrace/StrictMode measurement of the remaining
Application.onCreate()cost — the sandbox has no emulator. The claim this PR makes is that the named operations no longer run on the caller's thread, which the new Android test asserts directly. It does not claim a measured ANR reduction on the customer's device.Two behaviour changes worth a reviewer's eye:
close(), calls are buffered again rather than dropped, so a latersetup()replays them. The buffer is cleared onclose(), so nothing crosses a close/setup cycle from before it.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Fully autonomous
setup()to a background thread. It is the obvious reading of "move the work off the caller's thread", and it is wrong — the integration install loop registers the Activity lifecycle callbacks, so an async default silently loses the first screen view and the deep link on every cold start. The split above is the result: defer only what nothing in setup reads, and make the buffer carry the rest.:posthog-android:testDebugUnitTestneeds a UTF-8 locale or it passes every test and then fails writing its HTML report.Created with PostHog Desktop from this inbox report.