fix(analytics): guard polling start so concurrent callers don't leak executors (SDK-81)

[tylerjroach]

Summary

startPollingForDefinitions had no lock around the create-and-assign of pollingExecutor. Two concurrent callers would each allocate a fresh ScheduledExecutorService, the field would point at the second, and the first executor's worker thread + scheduled task would silently leak (still alive, still polling, no longer reachable for shutdown).

Add a private pollingLock guarding the executor's lifecycle:

• startPollingForDefinitions: under the lock, bail early if pollingExecutor != null (idempotent), otherwise create + schedule.
• stopPollingForDefinitions: snapshot the executor under the lock and clear the field, then run shutdown / awaitTermination outside the lock so a long shutdown can't block a concurrent start for 5s.

Context

Linear: SDK-81. Same audit-driven cross-SDK push; Java is one of three affected (Java, Ruby, Node). Python uses a not self._polling_task check that's safe because both async and sync paths share it under the GIL; Go uses CompareAndSwap.

Test plan

• All 54 LocalFlagsProviderTest tests pass (mvn test)
• New testConcurrentStartPollingDoesNotLeakExecutors spins up 8 contender threads behind a CountDownLatch start gate. Counts JVM threads named mixpanel-flags-poller before and after; asserts exactly one new poller exists, then asserts stop cleans it up. Before this fix the count would be 8.

🤖 Generated with Claude Code

[linear-code]

SDK-85

SDK-81

[tylerjroach]

@greptileai

[greptile-apps]

Confidence Score: 5/5

The change is safe to merge — it introduces a private monitor lock with a narrow critical section and no new external dependencies.

The locking strategy is correct: the critical section is small (just the null-check and assignment), shutdown is done outside the lock to avoid blocking concurrent starts, and the closed flag interaction is unchanged. Tests exercise both the concurrent-start and zero-interval edge cases.

No files require special attention.

Important Files Changed

Filename	Overview
src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java	Adds pollingLock monitor guarding executor create/assign and null/shutdown; also adds pre-lock interval validation. Correctly snapshots executor outside lock before shutdown.
src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java	Adds two new tests: concurrent-start leak guard (8 threads behind a CountDownLatch) and zero-interval wedge regression; both verify correct JVM thread counts.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant T1 as Thread 1
    participant T2 as Thread 2
    participant Lock as pollingLock
    participant Exec as pollingExecutor

    T1->>T1: fetchDefinitions()
    T2->>T2: fetchDefinitions()
    T1->>Lock: acquire lock
    Lock-->>T1: acquired
    T1->>Exec: null check - YES, create and schedule
    T1->>Lock: release
    T2->>Lock: acquire lock
    Lock-->>T2: acquired
    T2->>Exec: null check - NO, return early
    T2->>Lock: release

    Note over T1,Exec: stop path
    T1->>Lock: acquire lock
    Lock-->>T1: acquired
    T1->>Exec: snapshot executor, set null
    T1->>Lock: release
    T1->>Exec: shutdown outside lock
    T1->>Exec: awaitTermination 5s

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant T1 as Thread 1
    participant T2 as Thread 2
    participant Lock as pollingLock
    participant Exec as pollingExecutor

    T1->>T1: fetchDefinitions()
    T2->>T2: fetchDefinitions()
    T1->>Lock: acquire lock
    Lock-->>T1: acquired
    T1->>Exec: null check - YES, create and schedule
    T1->>Lock: release
    T2->>Lock: acquire lock
    Lock-->>T2: acquired
    T2->>Exec: null check - NO, return early
    T2->>Lock: release

    Note over T1,Exec: stop path
    T1->>Lock: acquire lock
    Lock-->>T1: acquired
    T1->>Exec: snapshot executor, set null
    T1->>Lock: release
    T1->>Exec: shutdown outside lock
    T1->>Exec: awaitTermination 5s

Loading

_{Reviews (3): Last reviewed commit: "fix(flags): validate pollingIntervalSeco..." | Re-trigger Greptile}

[tylerjroach]

Pushed 6116ece — region comment now reads SDK-81.

[Copilot]

Pull request overview

This PR fixes a concurrency issue in LocalFlagsProvider.startPollingForDefinitions() where concurrent callers could create multiple ScheduledExecutorService instances, leaking poller threads/tasks. It introduces a dedicated lock to make polling startup idempotent and adjusts shutdown to avoid holding the lock during potentially slow termination.

Changes:

• Add a private pollingLock to serialize polling executor lifecycle operations and make startPollingForDefinitions() idempotent when polling is already running.
• Refactor stopPollingForDefinitions() to snapshot-and-clear the executor under lock, then shutdown/await termination outside the lock.
• Add a concurrency regression test that races multiple startPollingForDefinitions() callers and asserts only one poller thread is created and later cleaned up.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/main/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProvider.java	Adds locking around poller executor creation/shutdown to prevent executor/thread leaks under concurrent starts.
src/test/java/com/mixpanel/mixpanelapi/featureflags/provider/LocalFlagsProviderTest.java	Adds a multithreaded regression test to confirm concurrent starts don’t create multiple poller threads and that stop cleans up.

[ketanmixpanel]

TAL at copilot comment. Rest looks good