Implement TcpStream::connect_timeout - #43062
Conversation
sfackler
added
the
T-libs-api
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Implementation looks good to me! I'm not too worried about a single-syscall rule here, TcpStream::connect already violates that anyway. This seems fine to me to land as unstable as well, want to open a tracking issue for this?
Also, can you add a test for a zero-duration being an error, as well as a sub-millisecond duration basically not panicking?
There was a problem hiding this comment.
Could you increase this to ~2s just to be super extra sure?
|
Oh also, I think tidy is failing |
aidanhs
added
the
S-waiting-on-author
|
@bors r=alexcrichton |
|
📌 Commit 82efdaa has been approved by |
|
🔒 Merge conflict |
|
@bors r=alexcrichton |
|
📌 Commit b9eb88d has been approved by |
|
⌛ Testing commit b9eb88d8a0a7158e287f262964492d0c95c038ce with merge 00312896f0a9d560627e63e590d6ff7e7e27a454... |
|
💔 Test failed - status-appveyor |
This breaks the "single syscall rule", but it's really annoying to hand write and is pretty foundational.
|
@bors r=alexcrichton |
|
📌 Commit 8c92da3 has been approved by |
Implement TcpStream::connect_timeout This breaks the "single syscall rule", but it's really annoying to hand write and is pretty foundational. r? @alexcrichton cc @rust-lang/libs
|
☀️ Test successful - status-appveyor, status-travis |
|
Why check the time out for zeros after initiating the connect? Thanks |
|
We typically do that because zero length durations do not behave as
expected in many APIs. I checked the documentation for poll and select and
it does behave here so we could remove the timeout. A zero length timeout
is kind of useless tin this method though. There's almost no chance the
connection would have finished in time.
…On Sat, Jul 15, 2017 at 11:35 AM setharnold ***@***.***> wrote:
Why check the time out for zeros *after* initiating the connect?
Thanks
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#43062 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABY2UQoMAcZDLYwd-sV01npvSWt8IS-6ks5sOQZmgaJpZM4ON66w>
.
|
5 participants
This breaks the "single syscall rule", but it's really annoying to hand
write and is pretty foundational.
r? @alexcrichton
cc @rust-lang/libs