fix counting of incoming streams #89
Conversation
session.go
Outdated
| @@ -850,6 +847,12 @@ func (s *Session) deleteStream(id uint32) { | |||
| return | |||
| } | |||
| s.memoryManager.ReleaseMemory(str.memory) | |||
| if s.client == (id%2 == 0) { | |||
| if s.numIncomingStreams == 0 { | |||
| panic("yamux BUG: numIncomingStream would be negative") | |||
There was a problem hiding this comment.
This panic is not caught by our panic handlers if it was caused by the application calling Close or Reset on the stream. Not really sure what the best thing to do here is. We want to surface this bug, but we can't use the normal logger for that. Every user of this library will have logging turned off, because the amount of things we log is just crazy.
There was a problem hiding this comment.
lets just ERROR log BUG very loudly, panic is innapropriate for our our applications.
There was a problem hiding this comment.
we should just change the logging to use go-log, it is very much forked already.
There was a problem hiding this comment.
Change to use the normal logger now, will change to go-log in a follow-up PR.
|
Maybe we should also remove most of the logging we have here. I haven't encountered a single case where it was useful. |
marten-seemann
force-pushed
the
fix-incoming-stream-counting
branch
from
1e8dd76 to
b8eae95
Compare
marten-seemann
force-pushed
the
fix-incoming-stream-counting
branch
from
b8eae95 to
b1d5347
Compare
marten-seemann
force-pushed
the
fix-incoming-stream-counting
branch
from
b1d5347 to
a51e116
Compare
session.go
Outdated
| if s.numIncomingStreams == 0 { | ||
| s.logger.Printf("[ERR] yamux: numIncomingStream underflow") | ||
| } | ||
| s.numIncomingStreams-- |
There was a problem hiding this comment.
else decrement, lets not make it negative.
Plus, we'll see all occurences.
The existing code had a uint32 underflow, because we were decrementing the
numIncomingStreamseven if the stream was already removed from the map.