Skip to content

fix: race conditions in ble observer#98

Merged
OleRoss merged 3 commits intomainfrom
fix/race-conditions-in-observer
Jan 20, 2026
Merged

fix: race conditions in ble observer#98
OleRoss merged 3 commits intomainfrom
fix/race-conditions-in-observer

Conversation

@OleRoss
Copy link
Copy Markdown
Contributor

@OleRoss OleRoss commented Jan 20, 2026

Summary

Right now the BleObserver is not thread save. There are multiple issues where especially around the synchronization of start/stop and the enumeration of subscriptions. This adds regression tests and attempts to make the BLE observer thread safe

Changes

  • ensure the handlers cannot be changed while iterating
  • Removing the lock while iterating
  • Documentation of edge cases

Testing

Added regression tests

Checklist

  • PR is scoped and not oversized
  • Documentation (and changelog) is updated if needed
  • CI Pipeline is successful
  • Self-review was done

@OleRoss OleRoss changed the title fix/race-conditions-in-observer fix: race conditions in ble observer Jan 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Repo Coverage

Line Coverage Branch Coverage


PR Coverage

53/64 changed lines covered (Lines: 82.81%, Branches: 75%)

Darp.Ble

🔴 Darp.Ble/Implementation/BleObserver.cs41/48 changed lines covered (Lines: 85.42%, Branches: 75%)
...
 84return;
 85 🟨             if (_observerState is not ObserverState.Stopped)
 86 🟥                 throw new InvalidOperationException($"Observer is in invalid state {_observerState}");
 87 ⬛ 
...
139// Best-effort only. No thread safety guarantees
140 🟨         if (_bleDevice.IsDisposing || _observerState is ObserverState.Stopping or ObserverState.Stopped)
141 🟥             return;
142 ⬛ 
...
176return;
177 🟨             if (_observerState is not ObserverState.Observing)
178 🟥                 throw new InvalidOperationException($"Observer is in invalid state {_observerState}");
179 ⬛ 
...
185 🟩             }
186 🟥             catch (Exception e)
187{
188 🟥                 Logger.LogObserverErrorDuringStopping(e);
189// In case of an error when stopping we assume we are still observing
190// Not ideal, but better than to wait for ever
191 🟥                 _observerState = ObserverState.Observing;
192 🟥                 throw;
193}
...
🔴 Darp.Ble/Utils/Helpers.cs11/14 changed lines covered (Lines: 78.57%, Branches: 75%)
...
 10 🟩         int handlerIndex = Array.IndexOf(array, item);
 11 🟨         if (handlerIndex < 0)
 12{
 13 🟥             newArray = null;
 14 🟥             return false;
 15}
...
 25 🟩         newArray = new T[arraySpan.Length - 1];
 26 🟨         if (handlerIndex > 0)
 27 🟥             arraySpan[..handlerIndex].CopyTo(newArray);
 28 🟩         if (handlerIndex < arraySpan.Length - 1)
...
🔴 Darp.Ble/BleObserverExtensions.cs1/2 changed lines covered (Lines: 50%, Branches: n/a)
...
 30{
 31 🟥             IDisposable unhook = bleObserver.OnAdvertisement(onAdvertisement: observer.OnNext);
 32 ⬛ 
...

Legend: 🟥 uncovered · 🟨 partial branch · 🟩 covered · ⬛ non-executable/blank
Generated by coverage-pr-comment

@OleRoss OleRoss merged commit 97bd796 into main Jan 20, 2026
2 of 3 checks passed
@OleRoss OleRoss deleted the fix/race-conditions-in-observer branch January 20, 2026 18:10
@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants