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

Channel API design question: semantics for aclose and/or send_end_of_channel #1110

Open
njsmith opened this issue Jun 16, 2019 · 1 comment

Comments

@njsmith
Copy link
Member

njsmith commented Jun 16, 2019

What's the question?

Some Channel objects allow multiple tasks to call send at the same time. Not all, but some. This creates complications for close/shutdown semantics.

Suppose that task A is blocked in await chan.send(...), and task B calls await chan.aclose(). What happens? In other contexts (like Streams, or tasks blocked in await chan.receive()), the answer is that the close happens immediately, and the send raises ClosedResourceError.

What if task B instead calls await chan.send_end_of_channel()? (Like Stream.send_eof, but for channels.) It has send in the name, so intuitively I guess it should block until any other tasks that are already blocked in send have unblocked (but any future sends will raise ClosedResourceError).

StapledStream.send_eof is implemented by calling self.send_stream.aclose. For streams, you aren't allowed to call send_eof if another task is calling send_all, so if this situation happens then it's a bug in the calling code and not our fault if something weird happens. (I guess there is some mild weirdness where if StapledStream.send_eof is cancelled then self.send_stream is still closed, because of aclose's special cancellation semantics... maybe I should open another issue to talk about that, but it doesn't seem like a huge deal.) But: what about StapledChannel.send_end_of_channel? If it's implemented by calling self.send_channel.aclose, then does it somehow wait for other tasks to finish calling self.send_channel.send before taking effect, or what?

What should we do?

I considered maybe saying: for Channels that allow support multiple simultaneous senders, then aclose should act like send_end_of_channel (i.e., wait for previous senders before closing), unless cancelled, in which case it should interrupt all senders and force an immediate close. (This would follow from our usual "forceful close" rules.) This actually makes it even less usable by StapledChannel though.

For StapledChannel, I guess we could wrap send and send_end_of_channel in a StrictFIFOLock, so send_end_of_channel automatically waits for all previous sends to complete. At least, so long as no-one else accesses the underlying channel directly. And this isn't very natural if we have a channel that doesn't support multiple simultaneous senders. (E.g., a framing channel wrapper around a stream.) In that case it's probably better to error out if two tasks try to send on the same channel at the same time, because if we blindly use a Lock then it'll mostly work until one of those tasks gets cancelled and it corrupts the channel's internal state, and then the other tasks are broken. Locks aren't really sufficient here; if your underlying transport is fragile like this, you usually need some kind of background task to mediate access.

Also, it's not clear that there are any cases where it's useful to have aclose or send_end_of_channel automatically wait politely. If you have multiple tasks calling chan.send at the same time, then that's because each send is essentially independent of the others. But if you want to close the channel, that inherently requires coordination with all the sending tasks.

If you've checked and made sure that all the tasks are done with the channel and won't call send again, then it doesn't matter which semantics aclose has, because there won't be any other tasks blocked in send. If you haven't checked and are just blindly calling aclose (e.g. some sort of emergency shutdown), then it doesn't matter which semantics aclose has, because you don't care whether there are any other tasks blocked in send. The only case where it matters is if you've checked, and made sure that all other tasks are done calling send except for the tasks that are making exactly one more call and you've somehow checked that they aren't about to make one more call, but have actually started the call and gotten to the point of being blocked. But this is like... impossible to check, and inherently fragile because small changes in the sending tasks could make it so that you call aclose just a moment before they get blocked in send, instead of a moment after. And the same arguments apply against send_end_of_channel...

So... I'm thinking that we should keep things simple: aclose acts like it does for other resources and immediately breaks any pending operations with ClosedResourceError. And, there is no send_end_of_channel. That way we don't have to figure out how to implement it for StapledStream, and we don't have to figure out what to call it either.

The one case I can think of where this would be limited is: if you have a channel that's used by only one task, but is used bidirectionally. So like: a task sends 10 objects, and then wants to do send_end_of_channel to notify the other side that it's done, and then it calls receive and waits for the other side to send some reply back.

Maybe we could have a aclose_send_half or something, so it doesn't imply that it has send-like semantics? Maybe we shouldn't worry about it for now, and wait to see how annoying it is to work around in practice?

@njsmith
Copy link
Member Author

njsmith commented Jun 17, 2019

I guess another use case for aclose_send_half would be if we make memory channels bidirectional by default (#719 (comment)), and someone wanted a unidirectional channel, and wasn't happy with using mypy to enforce that statically, then you could enforce it at runtime by opening a bidirectional channel and then calling aclose_send_half.

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

2 participants