-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revamp - Update libp2p-utp to follow latest interface-connection and interface-transport #74
Conversation
daviddias
commented
Feb 7, 2018
|
||
// TODO Unfortunately utp has no notion of gracious socket closing | ||
// This feature needs to be shimmed on top to make it a proper libp2p | ||
// transport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This continues to be the crux to support utp. libp2p currently expect gracious connection closing to be supported. We either remove this requirement or add a shim to libp2p-utp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/bittorrent/libutp/blob/31103141c4101bc05bfe4c622cb77d17ff90c0f1/utp.h#L176
Would exposing that function via the utp-native module fix this issue?
Edit: Looks like it would: https://github.com/bittorrent/libutp/blob/abbd3df4e30f071606a616e0139251470e2c85d8/utp_internal.cpp#L3349-L3382
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: Calling socket.end()
might fix it... investigating
https://github.com/mafintosh/utp-native/blob/master/src/socket_wrap.cc#L183-L186
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does send fin, but for some reason only from 1 side
utp_traffic.pcapng.gz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another update: server.close()
never emits the close event.
Edit: Didn't look in the JS code, only C++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test/net-api.js:261: socket.end() // utp does not support half open
Just found this.
Merging as this PR gets it closer to the final state, however the issue with gracious closing is still present. Tracking it here #19 |