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

Provide Channel.invokeOnClose { ... } function #341

Closed
LouisCAD opened this issue Apr 20, 2018 · 10 comments
Closed

Provide Channel.invokeOnClose { ... } function #341

LouisCAD opened this issue Apr 20, 2018 · 10 comments
Assignees

Comments

@LouisCAD
Copy link
Contributor

I'm currently writing an Android library that maps Bluetooth Low Energy scan callbacks to a ReceiveChannel.
I'd like to be able to detect when the channel becomes closed for receive and for send so I can stop the scan, but it didn't find any onClose { ... } function on Channel. Is there a reason it is not present, and could it be added in the future?

@Groostav
Copy link
Contributor

Groostav commented Apr 20, 2018

I believe the semantics are such that channel.receive() will suspend until either an elements becomes available or the channel is closed, with the latter resulting in the receive method getting an exception.

In this sense, your onClose code can happen like this:

val normalResult = try { someChannel.receive() }
catch(ex: ClosedReceiveChannelException){
  //onClose()
  throw ex
}

//regular flow

similarly, you can use receiveOrNull to replace try-catch syntax with null-coercion.

Also, the many coroutine factories like produce or actor will build-in some channel-closing logic, meaning you wont have to manually fire the channel.close() signal yourself which, in my use cases, has been the source of some annoying debugging.

@jcornaz
Copy link
Contributor

jcornaz commented Apr 21, 2018

@Groostav your proposition need to consume the channel. What if we want to have an onClose without consuming elements?

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Apr 22, 2018

I'd like to be able to detect when the channel becomes closed for receive and for send so I can stop the scan

Why is it not possible (or maybe not so convenient?) to catch ClosedForSendException and stop the pipeline from producing side?

More use-cases would be appreciated.
The biggest problem with this feature is to maintain same performance and footprint of the channel when no listeners are present and ideally the same performance when they are. This is (probably) why Go recommends to use a separate channel for cancellation

@SalomonBrys
Copy link

Here is my use case.
I am trying to register a BroadcastReceiver in Android to receive some events from the system (WiFi status).
Because I am mainly using coroutines, I want these events to be pushed in a channel.

I want the Activity to receive events from this channel but, when leaving the activity, when I cancel the ReceiveChannel, I want the receiver to be unregistered.
But I don't want to wait for the next event to arrive so that the sender gets a ClosedForSendException. The next event may arrive a lot later, and I don't want the receiver to be registered once the activity ends.

@LouisCAD
Copy link
Contributor Author

LouisCAD commented Apr 26, 2018

@SalomonBrys You can do it using produce as done here:
https://github.com/Beepiz/BleScanCoroutines/blob/24c8637fcef3ace262eb0b8aae5e64dc04d16dd0/core/src/main/java/com/beepiz/bluetooth/scancoroutines/experimental/BleScanner.kt#L35-L49
If the returned channel is closed, it causes either the for loop in the SuspendIterable or the send(…) call to throw the exception, leading to the finally block where you stop the scan, unregister what you need or release any resource.

@LouisCAD
Copy link
Contributor Author

@qwwdfsad I think we have the use cases (BLE scan, BroadcastReceiver to channel…), the label should be removed, right?

@SalomonBrys
Copy link

@LouisCAD OK, so that works, but it still feels like an elegant hack.
Elegant because the use of try-finally makes it very understandable.
Hacky because we are creating a producer that literally does nothing but transfering from a channel to another.
Maybe add a small extension function to "endorse" the use case.
Also, this seems a fairly regular use case. Is it documented?

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Apr 27, 2018

During discussion about this feature we've decided that adding Job to a channel (which alone resolves a lot of existing problems) kind of auto-implements this feature via channel.job.invokeOnCompletion.
Will be fixed with #260

@qwwdfsad qwwdfsad self-assigned this May 16, 2018
qwwdfsad added a commit that referenced this issue May 16, 2018
@elizarov elizarov changed the title Provide Channel.onClose { ... } function Provide Channel.invokeOnClose { ... } function Jun 26, 2018
@elizarov
Copy link
Contributor

Just a status update here. An attempt to add Job to all the channels (#260) is sinking. We could not find a satisfactory way to merge Job and Channel state machines. The backup plan it to still introduce Channel.invokeOnClose { ... } operation.

@elizarov
Copy link
Contributor

I've explained a workaround solution to the missing invokeOnClose in this SO answer: https://stackoverflow.com/questions/51029737/handle-cancelation-inside-kotlin-coroutines-producer/51041980#51041980

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants