Skip to content

Disconnect a socket If Foundry meets EOF while reading the socket#309

Merged
sgkim126 merged 2 commits intoCodeChain-io:masterfrom
majecty:f/hup-aborted
Apr 9, 2020
Merged

Disconnect a socket If Foundry meets EOF while reading the socket#309
sgkim126 merged 2 commits intoCodeChain-io:masterfrom
majecty:f/hup-aborted

Conversation

@majecty
Copy link

@majecty majecty commented Apr 8, 2020

This PR fixes #308

There are a few things to do more.

  1. Should we catch the error in the places?
  2. What should we do when we met other IO errors? Should we disconnect the socket?
  3. What happens if we write bytes to a disconnected socket? (This PR only checks the read call).

@majecty majecty added the do-not-merge Do not merge this PR label Apr 8, 2020
@majecty majecty requested a review from sgkim126 April 8, 2020 07:04
Copy link
Contributor

@sgkim126 sgkim126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@majecty

  1. I think it's fine.

  2. We should not close the connection if the error is WouldBlock or Interrupted.
    I'm not sure whether WriteZero will occur in the non-blocking socket.
    In my opinion, InvalidInput, InvalidOutput or TimeOut will not occur in our code.
    We'd better handle ConnectionReset and BrokenPipe, but I'm not sure there is a hup event or not in these cases.
    I think other errors will not occur while sending/receiving data.

  3. It's good if we can detect an error, but I suspect that the underlying implementation returns WouldBlock because we use a non-blocking socket. If it does, we cannot detect whether the connection has closed or there is just congestion.

@majecty
Copy link
Author

majecty commented Apr 8, 2020

@sgkim126 Thanks I'll develop this PR.

I found these sentences from the Write document.

A return value of 0 typically means that the underlying object is no longer able to accept bytes and will likely not be able to in the future as well, or that the buffer provided is empty.

How about disconnecting the socket if the write function returns 0?

https://doc.rust-lang.org/std/io/trait.Write.html

When I tested the situation, the write call emits "Broken Pipe" error. I'll check it more.

@majecty
Copy link
Author

majecty commented Apr 8, 2020

I found this article from ibm.com
https://developer.ibm.com/technologies/linux/tutorials/l-sockpit/

The closure of a peer socket can also be detected with the write API function. In this case, you’ll receive a SIGPIPE signal or, if this signal is blocked, the write function will return a -1 and set errno to EPIPE.

@majecty majecty changed the title [WIP] Disconnect a socket If Foundry meets EOF while reading the socket Disconnect a socket If Foundry meets EOF while reading the socket Apr 8, 2020
@majecty majecty removed the do-not-merge Do not merge this PR label Apr 8, 2020
@majecty majecty requested a review from MSNTCS April 8, 2020 09:15
@majecty
Copy link
Author

majecty commented Apr 8, 2020

@MSNTCS This RP fixes the mio issue.

@majecty
Copy link
Author

majecty commented Apr 8, 2020

@sgkim126 I'm ready for your review.

@sgkim126 sgkim126 merged commit 36bd178 into CodeChain-io:master Apr 9, 2020
@sgkim126 sgkim126 added the bug Something isn't working label Apr 9, 2020
@majecty majecty deleted the f/hup-aborted branch April 9, 2020 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot receive HUP event from mio >= 0.6.17

3 participants