Skip to content

fix: make integration installation atomic - #663

Merged
marandaneto merged 4 commits into
mainfrom
fix/atomic-integration-install-guards
Jul 29, 2026
Merged

fix: make integration installation atomic#663
marandaneto merged 4 commits into
mainfrom
fix/atomic-integration-install-guards

Conversation

@marandaneto

@marandaneto marandaneto commented Jul 28, 2026

Copy link
Copy Markdown
Member

💡 Motivation and Context

Concurrent setup of multiple PostHog instances can race through the @Volatile read-then-write guards used by default integrations. @Volatile guarantees visibility but not an atomic check-and-set, allowing duplicate legacy flushes, lifecycle callbacks, replay listeners, logcat threads, and other integration side effects.

Closes #662.

This also fixes the same pattern in the seven other affected integrations found during the repository-wide audit. Each integration tracks whether its instance won installation, preventing a losing instance from clearing the winner's shared state during teardown. Install and uninstall are synchronized per instance so remote-config updates cannot race with teardown side effects. Cached-event integrations that lose installation also shut down their unused executor.

💚 How did you test it?

  • Added concurrent-install regression tests for legacy event flushing, executor shutdown, and Android lifecycle callback registration.
  • Added coverage proving a non-owner teardown does not clear the winning installation.
  • Added explicit test isolation for the shared cached-events installation guard.
  • JAVA_TOOL_OPTIONS=-Dnet.bytebuddy.experimental=true make testJava
  • ./gradlew :posthog:apiCheck
  • Focused Android unit tests for all affected Android integrations, including session replay.
  • make checkFormat

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented with the Pi coding agent using repository inspection, shell, editing, and test tools. The audit covered every production @Volatile Boolean and limited the change to the eight integrationInstalled guards that perform a check-and-set. Review feedback led to owner-aware teardown and per-instance synchronization across those guards; other volatile fields remain unchanged because they are visibility-only or already lock/thread confined.

@marandaneto marandaneto self-assigned this Jul 28, 2026
@marandaneto
marandaneto marked this pull request as ready for review July 28, 2026 18:06
@marandaneto
marandaneto requested a review from a team as a code owner July 28, 2026 18:06
@marandaneto

Copy link
Copy Markdown
Member Author

setup calls install so likely not happening because setup is behind a lock, but its good to fix it anyway

@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
### Issue 1
posthog-android/src/main/java/com/posthog/android/internal/PostHogActivityLifecycleCallbackIntegration.kt:97
**Non-owner teardown clears installation**

When separate PostHog instances concurrently install this integration, the instance that loses the install CAS still clears the shared flag and unregisters its own unregistered callback during shutdown. This releases the winning instance's ownership, can trigger an invalid callback removal, and allows a later setup to register a duplicate callback; the same ownership error affects the sibling integrations using this shared guard pattern.

### Issue 2
posthog/src/main/java/com/posthog/errortracking/PostHogErrorTrackingAutoCaptureIntegration.kt:57-60
**Handler state races with teardown**

When a remote-config installation overlaps `close()`, installation marks the integration active before registering the process-wide handler. Teardown can clear the flag and restore the previous handler between those operations, after which installation registers this handler while the flag remains false, leaving exception autocapture active after shutdown.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix: make integration installation atomi..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-07-29 09:20:30 UTC
Duration: 118593ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 405ms
Format Validation.Event Has Uuid 40ms
Format Validation.Event Has Lib Properties 35ms
Format Validation.Distinct Id Is String 29ms
Format Validation.Token Is Present 28ms
Format Validation.Custom Properties Preserved 29ms
Format Validation.Event Has Timestamp 32ms
Retry Behavior.Retries On 503 7030ms
Retry Behavior.Does Not Retry On 400 4029ms
Retry Behavior.Does Not Retry On 401 4028ms
Retry Behavior.Respects Retry After Header 7030ms
Retry Behavior.Implements Backoff 17037ms
Retry Behavior.Retries On 500 7019ms
Retry Behavior.Retries On 502 7020ms
Retry Behavior.Retries On 504 7022ms
Retry Behavior.Max Retries Respected 17037ms
Deduplication.Generates Unique Uuids 54ms
Deduplication.Preserves Uuid On Retry 7018ms
Deduplication.Preserves Uuid And Timestamp On Retry 12040ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7019ms
Deduplication.No Duplicate Events In Batch 43ms
Deduplication.Different Events Have Different Uuids 28ms
Compression.Sends Gzip When Enabled 21ms
Batch Format.Uses Proper Batch Structure 20ms
Batch Format.Flush With No Events Sends Nothing 15ms
Batch Format.Multiple Events Batched Together 36ms
Error Handling.Does Not Retry On 403 4022ms
Error Handling.Does Not Retry On 413 4022ms
Error Handling.Retries On 408 5031ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 35ms
Request Payload.Flags Request Uses V2 Query Param 27ms
Request Payload.Flags Request Hits Flags Path Not Decide 25ms
Request Payload.Flags Request Omits Authorization Header 29ms
Request Payload.Token In Flags Body Matches Init 24ms
Request Payload.Groups Round Trip 26ms
Request Payload.Groups Default To Empty Object 23ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 23ms
Request Payload.Disable Geoip Omitted Defaults To False 25ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 24ms
Request Lifecycle.No Flags Request On Init Alone 14ms
Request Lifecycle.No Flags Request On Normal Capture 34ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 42ms
Request Lifecycle.Mock Response Value Is Returned To Caller 23ms
Retry Behavior.Retries Flags On 502 325ms
Retry Behavior.Retries Flags On 504 326ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 26ms

@marandaneto
marandaneto requested a review from a team July 29, 2026 07:04
@marandaneto
marandaneto enabled auto-merge (squash) July 29, 2026 09:16
@marandaneto
marandaneto merged commit 932118f into main Jul 29, 2026
16 checks passed
@marandaneto
marandaneto deleted the fix/atomic-integration-install-guards branch July 29, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PostHogSendCachedEventsIntegration: @Volatile guard is non-atomic — two concurrent install() calls both pass, flushing legacy events twice

3 participants