Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscribe/close throws exception #58

Closed
grubesa opened this issue Feb 12, 2020 · 3 comments
Closed

Subscribe/close throws exception #58

grubesa opened this issue Feb 12, 2020 · 3 comments

Comments

@grubesa
Copy link

grubesa commented Feb 12, 2020

Hi,
I think I found a bug. It occurs in following situation:
When I subscribe to characteristic with connection.openNotificationSubscription(notificationCharacteristic) and
connection closes before I get the chance to unsubscribe from characteristic. I get this exception:

kotlinx.coroutines.CoroutinesInternalError: Fatal exception in coroutines machinery for CancellableContinuation(DispatchedContinuation[Unconfined, Continuation at com.beepiz.bluetooth.gattcoroutines.extensions.ReceiveChannelKt$withCloseHandler$1.invokeSuspend(ReceiveChannel.kt:26)@8d3a248]){CompletedExceptionally[com.beepiz.bluetooth.gattcoroutines.ConnectionClosedException: The connection has been irrevocably closed.]}@93e38e1. Please read KDoc to 'handleFatalException' method and report this incident to maintainers
        at kotlinx.coroutines.DispatchedTask.handleFatalException$kotlinx_coroutines_core(Dispatched.kt:275)
        at kotlinx.coroutines.DispatchedTask.run(Dispatched.kt:246)
        at kotlinx.coroutines.EventLoop.processUnconfinedEvent(EventLoop.common.kt:65)
        at kotlinx.coroutines.DispatchedKt.resumeUnconfined(Dispatched.kt:389)
        at kotlinx.coroutines.DispatchedKt.dispatch(Dispatched.kt:295)
        at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:250)
        at kotlinx.coroutines.CancellableContinuationImpl.completeResume(CancellableContinuationImpl.kt:327)
        at kotlinx.coroutines.channels.AbstractChannel$ReceiveHasNext.resumeReceiveClosed(AbstractChannel.kt:965)
        at kotlinx.coroutines.channels.AbstractSendChannel.helpClose(AbstractChannel.kt:332)
        at kotlinx.coroutines.channels.AbstractSendChannel.close(AbstractChannel.kt:271)
        at kotlinx.coroutines.channels.ArrayBroadcastChannel$Subscriber.checkOffer(ArrayBroadcastChannel.kt:259)
        at kotlinx.coroutines.channels.ArrayBroadcastChannel.checkSubOffers(ArrayBroadcastChannel.kt:127)
        at kotlinx.coroutines.channels.ArrayBroadcastChannel.close(ArrayBroadcastChannel.kt:66)
        at com.beepiz.bluetooth.gattcoroutines.GattConnectionImpl.closeInternal(GattConnectionImpl.kt:141)
        at com.beepiz.bluetooth.gattcoroutines.GattConnectionImpl.close(GattConnectionImpl.kt:120)

While investigating, I realised that you attach close handler to notifications channel which causes the exception:

notificationChannel.withCloseHandler {           
    setCharacteristicNotificationsEnabled(characteristic, enable = false) 
}

setCharacteristicNotificationsEnabled(characteristic, enable = false) will fail if device is not connected.
I also realise that you have disableNotificationsOnChannelClose flag which could handle this case, but I don't think that would be the right way to handle my use case. In my case, remote device suddenly initiates (unexpected) connection close which triggers this exception.
I think this exception shouldn't happen, library should check if connection is alive before calling setCharacteristicNotificationsEnabled(characteristic, enable = false).
I posted a PR with my proposed solution.
You can reproduce this bug very easily:

  1. Open subscription on characteristic
  2. Close connection before unsubscribing from characteristic
@LouisCAD
Copy link
Collaborator

LouisCAD commented Mar 3, 2020

Hi, I'm sorry for the late reply, but here it is:

Checking the connection is alive before doing something can be a fail-fast optimization, but other than that, it cannot prevent sudden connection drops, so checking the connection somehow before using it would only reduce occurrences of that behavior.

I'll take a look at your PR soon, still.

@dleppik
Copy link

dleppik commented Mar 3, 2020

For what it's worth, @grubesa's change was a big help to me. I have a situation where the device can suddenly go out of range, so graceful disconnects are not an option.

@LouisCAD
Copy link
Collaborator

Hello, I finally released a new version (after almost 3 years 😓 ), it's on MavenCentral and should avoid un-preventable crashes like this one.
I'll let you read the (short) release notes of the version 0.5.0 to learn a bit more before you update.

BTW, BleGattCoroutines is, as you'd expect, now on MavenCentral.

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

No branches or pull requests

3 participants