Add the ability to close a queue.

[Fuyukai]

No description provided.

[sorcio]

I'm not sure about this change. The one part I'm specifically worried about is what is supposed to happen when the queue still holds data in its buffer. In this implementation, the getters never get a chance to consume that data. I'm not entirely sure of the full range of use cases for this API, but I can imagine that some might instead want the queue to be closed on the consuming side only when it's empty.

[codecov]

Codecov Report

Merging #573 into master will decrease coverage by 4.48%.
The diff coverage is 84%.

@@            Coverage Diff             @@
##           master     #573      +/-   ##
==========================================
- Coverage   99.27%   94.78%   -4.49%     
==========================================
  Files          89       95       +6     
  Lines       10628    13410    +2782     
  Branches      747     1495     +748     
==========================================
+ Hits        10551    12711    +2160     
- Misses         59      641     +582     
- Partials       18       58      +40

Impacted Files	Coverage Δ
trio/tests/test_sync.py	100% <100%> (ø)	⬆️
trio/_sync.py	95.79% <77.14%> (-4.21%)	⬇️
trio/_core/_io_epoll.py	0% <0%> (-100%)	⬇️
trio/_core/_windows_cffi.py	0% <0%> (-91.31%)	⬇️
trio/_core/_io_windows.py	0% <0%> (-77.1%)	⬇️
trio/_core/tests/test_windows.py	23.33% <0%> (-76.67%)	⬇️
trio/_core/tests/test_epoll.py	27.77% <0%> (-72.23%)	⬇️
trio/hazmat.py	72.72% <0%> (-27.28%)	⬇️
trio/_highlevel_open_unix_stream.py	66.66% <0%> (-11.12%)	⬇️
trio/_util.py	90.17% <0%> (-9.83%)	⬇️
... and 44 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd0d877...8a85421. Read the comment docs.

[smurfix]

@sorcio Seconded. A closed queue should behave like a closed file descriptor – you can read what's been sent before closing but you can't send new things.

[njsmith]

There's an ambiguity that's baked into Queue combining both the send and receive ends onto the same object. If you think of close as closing the put side, then it should be possible to get already-queued data. If you think of it as closing the get side, then it should discard existing data and wake up any putters with something like a BrokenQueueError.

One option would be to have separate close_get and close_put methods (though then you have to decide which, if any, is invoked by with queue:). Another would be to split the queue into two objects, as discussed in #497.

I think that probably in the long run we do want to refactor queue into a two-object design, but if you need this functionality urgently then maybe it'd be easiest to call it close_get (assuming that's the version you need)?

[Fuyukai]

The queue now has two methods:

• Queue.close_put - which closes the put side
• Queue.close_both_sides - which closes both the put and the get side, clearing the data buffer

Also, close_put closes the get side if there is no data in the buffer. Good idea, or not? Should I also make it so that if the queue is marked as closed on the put side, get automatically closes the queue on the get side if the buffer empties (because there's no point getting from an empty queue with no putters allowed).

[Fuyukai]

I made it so that the get side automatically closes if there's no data and the put side is closed, because otherwise it would let the queue block forever.

[Fuyukai]

(The failing Jenkins is because Python segfaulted.)

[sorcio]

@Fuyukai there are a few cases that you're not testing and I suspect there might be a bug there. I cannot thing straight enough right now but let me just drop some notes.

1. Why not two explicitly named flags? I found it hard to track what is 0 and what is 1. You can't use any() but a simple or will do its job (and is maybe faster?)

2. You don't check for closed state in the close_* methods. What happens if I close twice? What happens if I call one and then the other?

3. You don't clean up the _put_wait and _get_wait collections after you send an error to waiters. I suspect there might be a situation where another operation will try to get a waiter from the list and reschedule it and break everything because we already rescheduled it with the QueueClosed exception. Maybe you can add more tests where there are multiple concurrent getters/putters.

4. small typo in the comments (size/side)

[njsmith]

I'm going to sleep on this one... in the mean time, probably anyone interested in this will also be interested in #586 :-)

[njsmith]

@Fuyukai oh right, sorry that took longer than expected... thanks for your patience here.

[njsmith]

(Oh right, and the context for the record: this is being closed in favor of #586)