don't block sendQueue.Send() if the runloop already exited. #2656
Conversation
send_queue_test.go
Outdated
| q.Run() | ||
| close(done) | ||
| }() | ||
| q.Close() |
There was a problem hiding this comment.
Can we use a PacketConn that errors here, instead of calling close? That more closely mirrors the actual issue that you discovered, as the session only calls Close after calls to Send have returned.
There was a problem hiding this comment.
@marten-seemann Thanks, I updated this to have the MockConnection trigger an error on Write (how the queue sees that error) -- I'm not sure rigging in an actual PacketConn would add much in this context, but let me know.
There was a problem hiding this comment.
Thank you, that's actually what I meant. Should have been more precise.
This can lead to a deadlock where session.shutdown() never exits because it is blocked on a Send() but the sendQueue has exited due to a write error.
ltucker
force-pushed
the
shutdown_deadlock
branch
from
084edb0
to
3c1e597
Compare
Codecov Report
@@ Coverage Diff @@
## master #2656 +/- ##
==========================================
+ Coverage 86.42% 86.46% +0.05%
==========================================
Files 122 122
Lines 9761 9765 +4
==========================================
+ Hits 8435 8443 +8
+ Misses 990 988 -2
+ Partials 336 334 -2
Continue to review full report at Codecov.
|
This can lead to a deadlock where session.shutdown() never exits
because it is blocked on a Send() but the sendQueue has exited due to
a write error.
Fixes #2655 but needs consideration for other consequences ...