Skip to content

Commit 4a03d88

Browse files
refactor(audience-session): hoist Identity out of _initLock; double-seal in Shutdown
Two Cursor Bugbot findings on 63b13bb: SetConsent held _initLock across Identity.GetOrCreate / Identity.Get. On the None → Anonymous/Full upgrade path, GetOrCreate creates directories and writes a UUID file — real disk I/O under the lock. Moved the capture outside _initLock. Identity's own _sync lock serialises the I/O; TOCTOU on previous is accepted — a racing SetConsent fires its own PUT and our slightly-stale anonymousId still identifies the user. Shutdown's outside-lock EmitEndAndSeal targeted whatever _session pointed to at call time. A Reset completing fully (including new session Start()) in the narrow window before Shutdown's lock acquire would leave the new session's session_start on the wire with no matching session_end — Shutdown disposes the new session after flipping _initialized=false, so Track drops the closing event. Added a second EmitEndAndSeal inside the lock as a race guard. Idempotent on the same instance (no-op via _sessionId null check); the slow path only runs when the race actually happened. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 63b13bb commit 4a03d88

1 file changed

Lines changed: 27 additions & 8 deletions

File tree

src/Packages/Audience/Runtime/ImmutableAudience.cs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -374,16 +374,32 @@ public static void SetConsent(ConsentLevel level)
374374
{
375375
if (!_initialized) return;
376376

377+
var config = _config;
378+
if (config == null) return;
379+
380+
// Snapshot check before any I/O: no-op if already at target consent.
381+
var snapshotPrevious = _consent;
382+
if (level == snapshotPrevious) return;
383+
384+
// Capture anonymousId for the PUT audit trail outside _initLock.
385+
// Identity methods hold their own _sync lock; disk I/O on a cold
386+
// cache (None → Anonymous/Full upgrade creates the UUID file) does
387+
// not block _initLock. A racing SetConsent may change _consent
388+
// between this read and our lock acquire — acceptable, the racing
389+
// call fires its own PUT and our slightly-stale ID still
390+
// identifies the user.
391+
var anonymousIdForPut = snapshotPrevious == ConsentLevel.None
392+
? Identity.GetOrCreate(config.PersistentDataPath!, level)
393+
: Identity.Get(config.PersistentDataPath!);
394+
377395
// Phase 1 under _initLock: flip _consent and swap _session / _userId.
378396
// Phase 2 outside the lock runs the blocking side effects (persist,
379397
// dispose, purge, downgrade, backend sync, new session_start) so a
380398
// concurrent Shutdown / Init / Reset isn't held waiting on them.
381399
ConsentLevel previous;
382-
AudienceConfig? config;
383400
EventQueue? queue;
384401
Session? oldSession = null;
385402
Session? newSession = null;
386-
string? anonymousIdForPut;
387403
bool downgradeFullToAnonymous = false;
388404

389405
lock (_initLock)
@@ -397,12 +413,6 @@ public static void SetConsent(ConsentLevel level)
397413
previous = _consent;
398414
if (level == previous) return;
399415

400-
// Snapshot anonymousId before Identity.Reset (on None) wipes it.
401-
// The PUT audit trail needs to record whose consent changed.
402-
anonymousIdForPut = previous == ConsentLevel.None
403-
? Identity.GetOrCreate(config.PersistentDataPath!, level)
404-
: Identity.Get(config.PersistentDataPath!);
405-
406416
_consent = level;
407417

408418
if (level == ConsentLevel.None)
@@ -555,6 +565,15 @@ public static void Shutdown()
555565
{
556566
if (!_initialized) return;
557567

568+
// Race guard: a concurrent Reset or SetConsent(upgrade-from-None)
569+
// may have swapped _session to a new instance that has already
570+
// fired session_start. Seal it too so its session_end lands
571+
// before the flag flip. Idempotent on the same instance (no-op
572+
// via _sessionId null check); the slow path only runs when
573+
// Reset fully completed its Start() between the outside-lock
574+
// call above and this point — a narrow window.
575+
_session?.EmitEndAndSeal();
576+
558577
// Flip the gate. Init / SetConsent / Reset acquiring after
559578
// this see _initialized == false and return cleanly.
560579
_initialized = false;

0 commit comments

Comments
 (0)