Skip to content

fix: Synchronize provider state access on the state lock - #54

Open
kinyoklion wants to merge 1 commit into
mainfrom
devin/java-state-lock
Open

fix: Synchronize provider state access on the state lock#54
kinyoklion wants to merge 1 commit into
mainfrom
devin/java-state-lock

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Aug 19, 2026

Copy link
Copy Markdown
Member

getState() synchronized on the state field's value instead of the stateLock monitor, so reads were never mutually exclusive with writes.

  • getState() now locks stateLock, matching setState
  • initialize sets the ready state through setState and reads it through getState instead of touching the field unlocked
  • No behavior change for callers beyond correct visibility of state transitions

@cursor review

Implementation details

Root cause

synchronized (state) locks on the ProviderState enum constant currently referenced by the field. A reader holding the monitor of ProviderState.NOT_READY does not exclude a writer holding stateLock, so state transitions had no happens-before relationship with reads. Because enum constants are JVM-wide singletons, it also meant contending on a monitor shared with any other code that happens to lock the same constant.

initialize also wrote state = ProviderState.READY and later read state without any lock, on a different thread than the data source status listener that mutates it.

Alternatives considered

Making the field volatile would fix visibility, but the VALID branch of handleDataSourceStatus needs a compare-and-set over the field, so the lock is still required; keeping a single lock is simpler than mixing both.

Testing

./gradlew test — all 44 tests pass. The existing LifeCycleTest cases already assert the state transitions this lock protects (NOT_READYREADY, and → ERROR on a failed data source); a test cannot deterministically observe the previous incorrect locking, so no new test is added.

No visual preview applies — this is a server-side provider change.

Link to Devin session: https://app.devin.ai/sessions/0c452d209ec54b068ba120b4c92b8f6c
Requested by: @kinyoklion


Note

Overview
Fixes incorrect locking around the OpenFeature provider lifecycle state so reads and writes use the same monitor.

getState() now synchronizes on stateLock instead of on the ProviderState enum value held in the field, which did not exclude concurrent updates from setState and could contend on JVM-wide enum monitors. initialize routes the early-ready path through setState(READY) and the readiness check through getState() instead of assigning or reading the state field without the lock.

Callers should see the same transitions, with correct visibility and mutual exclusion when the data source listener updates state on another thread.

Reviewed by Cursor Bugbot for commit dfc2c89. Bugbot is set up for automated code reviews on this repo. Configure here.

Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
@kinyoklion kinyoklion self-assigned this Aug 19, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@kinyoklion
kinyoklion marked this pull request as ready for review August 19, 2026 17:16
@kinyoklion
kinyoklion requested a review from a team as a code owner August 19, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant