fix: Try improving relay_datagram_send_channel()
#3118
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes #3067
Since multiple
Connection
s can have multipleAsyncUdpSocket
IpPollers
, it's incorrect to assume that a singleAtomicWaker
can wake up these tasks correctly.Instead, if there's only one
AtomicWaker
for all of them, only the last registered waker will be woken when there's capacity again.I'm changing this such that
poll_writable
will actually add the current task's waker to a list, if it hasn't been added yet.And successfully
recv
ing an item will wake all tasks.A second issue was that between the call to
self.sender.capacity()
(it being 0) andself.waker.register
, another thread might have successfullyrecv
d an item and calledself.waker.wake()
.This means that the first thread could've registered a waker, while the capacity is actually non-zero. Not terrible (it's very likely that it'll be woken up when the capacity jumps to 2 this time), but also not great.
The fix is to re-check the capacity after registering the waker.
The downside is that we keep the waker and thus potentially get a spurious wakeup, but that's fine.
Notes & open questions
This... "fixes" the concerns, but I'd actually like to write a loom test for the concerns eventually.
It's just... a lot of work, and I'd rather prioritize other things, but also improve this part of the code at the same time.
Change checklist