Skip to content

Decide and apply a Toggle under one lock, so concurrent toggles each flip once [patch] - #31

Merged
matt-edmondson merged 2 commits into
mainfrom
fix/30-atomic-toggle
Oct 31, 2026
Merged

matt-edmondson merged 2 commits into
mainfrom
fix/30-atomic-toggle

Conversation

@matt-edmondson

Copy link
Copy Markdown
Collaborator

Fixes #30

What was wrong

KeepAwakeController.Toggle() read IsActive, called SetActive(!IsActive), and then read IsActive again, taking gate separately each time. When two threads toggled an inactive controller, both could read "off" and both ask for "on". Both calls then returned true, and the controller stayed on after two flips.

Change

  • SetActive now takes a Func<bool, bool> from the current state to the state wanted. It evaluates that inside the lock that applies the change, and returns the state it left.
  • Enable/Disable pass constant functions. Toggle passes current => !current and returns the result, instead of re-reading IsActive.
  • StateChanged is still raised after the lock is released.

Tests

  • FakeSleepBlocker gains an AcquireDelay that widens the race window.
  • New test Toggle_ConcurrentCallsEachFlipOnceAndReportTheirOwnResult: 200 trials of 4 threads released together by a Barrier. Each trial asserts the final state is N % 2 == 1, and that the return values contain exactly ⌈N/2⌉ true and ⌊N/2⌋ false.
    • Without the fix, it failed on trial 0 in 3 of 3 runs.
    • With the fix, it passed in 3 of 3 runs. The full suite passes (28/28).

Generated by Claude Code

…flip once [patch]

Toggle read IsActive, then called SetActive, then read IsActive again,
each under its own acquisition of the gate. Two threads toggling an
inactive controller could both read "off", both ask for "on", and both
return true, leaving it on after two flips.

SetActive now takes a function from the current state to the wanted one
and runs it inside the lock that applies the change, returning the state
it left. Toggle returns that instead of re-reading. Fixes #30.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4T6sUBiRNom98NuYi
@sonarqubecloud

Copy link
Copy Markdown

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.

Two concurrent Toggle() calls on KeepAwakeController can leave it active and both return true: the read and the flip happen under separate locks

2 participants