shinyorg / shiny

.NET Framework for Backgrounding & Device Hardware Services (iOS, Android, & Catalyst)
https://shinylib.net
MIT License
1.45k stars 227 forks source link

[Bug]: Unsubscription from characteristic notifications might create unstable state #1381

Closed TimofeyBurak closed 8 months ago

TimofeyBurak commented 9 months ago

Component/Nuget

BluetoothLE Client (Shiny.BluetoothLE)

What operating system(s) are effected?

Version(s) of Operation Systems

Android 8.0

Hosting Model

Steps To Reproduce

  1. Connect to peripheral
  2. Subscribe to notifications by calling NotifyCharacteristic(serviceUuid, chracteristicUuid)
  3. Unsubscribe from notifications while notifications are being enabled (in the middle of the async WriteDescriptor() execution)
  4. Read (should also work with wright) any characteristic right after unsubscription

Expected Behavior

Next operaation after unsubscription should be executed succesfully. Also, WriteDescriptor() operation, which enables the notifications, should run into completion. Then, since there are no subscribers, another WriteDescriptor() operation should be executed to disable the notifications.

NOTE: this expectation is based on general guidelines for working with BLE on Android, for example:

Actual Behavior

Shiny.BluetoothLE.BleException: Failed to read characteristic is thrown.

According to the NotifyCharacteristic(serviceUuid, chracteristicUuid) source code, async WriteDescriptor(descriptor, data, ct) will be cancelled if observable is unsubscribed in the middle of execution. According to the WriteDescriptor(descriptor, data, ct) source code, if cancelled - this method cancels the waiting for callback and throws exception.

So if cancellation of the WriteDescriptor(descriptor, data, ct) happens, then:

  1. Awaiting for the descriptor write callback will be cancelled, but Android OS will continue execution (and might be completed successfully);
  2. this.TryNotificationCleanup(characteristic, serviceUuid, characteristicUuid) will not be executed, as characteristic variable is not initialised at this point;
  3. SemaphoreOperationQueue will be released, so the next operation in the queue would be executed right away (if there are any waiting), or new operation could be scheduled right away. Either way, new operation could be executed at the same time as Android OS is still executing descriptor write.

Exception or Log output

Shiny.BluetoothLE.BleException: Failed to read characteristic: 0000001f-0000-1000-8000-00805f9b34fb at Shiny.BluetoothLE.Peripheral+<>cDisplayClass35_1.b1 (System.Threading.CancellationToken ct) [0x000d4] in <6b51d496651b43a9a8f0fce493c18768>:0 at Shiny.BluetoothLE.Intrastructure.OperationQueueExtensions+<>cDisplayClass0_1`1[T].b1 () [0x0007c] in <6b51d496651b43a9a8f0fce493c18768>:0 at Shiny.BluetoothLE.Intrastructure.SemaphoreOperationQueue.Queue (System.Func1[TResult] task, System.Threading.CancellationToken cancellationToken, System.String caller) [0x00157] in <6b51d496651b43a9a8f0fce493c18768>:0 at Shiny.BluetoothLE.Intrastructure.OperationQueueExtensions+<>c__DisplayClass0_01[T].b__0 (System.Threading.CancellationToken ct) [0x000cf] in <6b51d496651b43a9a8f0fce493c18768>:0

Code Sample

Due to NDA I cannot share the exact codebase we use in production, hope this simplified gist would be enough: https://gist.github.com/TimofeyBurak/169e3fb43d6741b998b4a64dfa678011

Code of Conduct

aritchie commented 9 months ago

It's a bit of a different use-case, but I can see how this can occur. I can work with the sample you've provided - thanks

aritchie commented 8 months ago

I have reordered a couple of things around, but I'm only going to go so far with this one since it is a very weird use-case. Notifications aren't meant to hook/unhook in a rapid fashion like this.