Opened 7 years ago
Last modified 5 years ago
#1040 open defect
Streaming half-close unused and broken
Reported by: | zzz | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | eventually |
Component: | streaming | Version: | 0.9.7.1 |
Keywords: | docs reliability | Cc: | Zlatin Balevsky |
Parent Tickets: | Sensitive: | no |
Description
Docs claim you can send a SYN and FIN in one packet (SYN and CLOSE in streaming terminology).
But we don't now. And it wouldn't work well if we did. If you sent a CLOSE it would start disconnect timers, e.g. Connection.setCloseSentOn(). Also, streams closed in one direction may cause closes in the other, e.g. in i2ptunnel stream forwarders.
So sending a CLOSE one direction but having the conn stay open for hours in the other direction does not work now.
Fixing this could eliminate the transmission of one packet (the CLOSE)
Subtickets
Change History (10)
comment:1 Changed 7 years ago by
Cc: | Zlatin Balevsky added |
---|
comment:2 Changed 7 years ago by
comment:4 Changed 7 years ago by
What happens when both sides send the first close at almost the same time? For example, Alice sends her close while Bob's close is in flight.
comment:5 Changed 7 years ago by
You mean how does it work now? Or with the #1042 fixes? Or after complete half-close implementation?
Each side keeps a local state. On each side, the important thing is whether you received a close before you sent a close, or not. Strictly speaking, it is whether the close you sent contained an ack of their close. Which it either did or it didn't.
In the #1042 fixes I'm not tracking that, just looking at before or after, so it could be better. But by being conservative about when to mark our close as sent, you can be pretty safe. You can always go down the left side of the 793 pic (to TIME-WAIT) if you aren't sure. The right side is just an optimization.
comment:6 Changed 7 years ago by
#1042 fixes checked in to i2p.i2p.zzz.test2 33d2af1d4e98089e8f260804254f5b5bca1dd0d5 fix many half-close bugs on the receive side. Probably more to do.
To actually set CLOSE in the SYN packet (or any last packet with data) requires changes in how MessageOutputStream?.close() propagates to ConnectionDataReceiver?.buildPacket(). Right now, MessageOutputStream?.getClosed() doesn't return true until MessageOutputStream?.destroy() is called. That's what ConnectionDataReceiver?.buildPacket() calls. It should just propagate through like an EOF.
Possibly related: #629
comment:12 Changed 7 years ago by
Keywords: | docs added |
---|
Docs should be fixed at http://www.i2p2.i2p/streaming to say the originator should NOT put the CLOSE in the SYN packet. Should be OK for the receiver. Right now we say in several places that you can combine SYN and CLOSE without a lot of detail.
As there's at least two people implementing streaming in C now we should get on this.
comment:13 Changed 5 years ago by
Keywords: | reliability added |
---|---|
Milestone: | 0.9.12 → eventually |
comment:14 Changed 5 years ago by
Status: | new → open |
---|
Maybe it isn't unused but it sure looks broken. Following code from 2005 in ConnectionPacketHandler?:
That looks horribly wrong.
Following shows a close sent (initiated by browser close) followed by us not acking retransmissions.
http://pastethis.i2p/show/AsaD9tGIr0VOhiCXgmsp/
Following shows us not getting our close acked:
http://pastethis.i2p/show/uR27CEYy18Mh3l5NjGKo/
Note that both pastes are from modified code, but I think it could happen in trunk. For further research.