GNOME Bugzilla – Bug 728928
Provide separate error code for "Connection reset by peer"
Last modified: 2015-06-05 12:22:21 UTC
All the internal processing hiding behind GInputStream is nice, but what I miss is better error checking. For example, a very specific error "Connection reset by peer" is returned as G_IO_ERROR_FAILED, which is... well... useless. It would be good to provide a "backend" specific error codes. For example, a GTcpConnection/GTlsConnection/GSocket/... should provide their own error domains and codes, thus if I receive G_IO_ERROR_GNUTLS, then I know the code is from the GnuTLS pool and I can distinguish between many fine-grained error codes. As an initial step, it would be nice to at least provide G_IO_ERROR_CONNECTION_RESET_BY_PEER and/or G_IO_ERROR_DISCONNECTED , beside existing G_IO_ERROR_CONNECTION_REFUSED.
*** Bug 737705 has been marked as a duplicate of this bug. ***
I had started working on this a while back, but it turns out that this is a huge mess, portability-wise, so we need to be careful about what error codes we add, and what we promise about them. In particular, whether you get EPIPE or ECONNRESET when writing to a closed socket appears to depend on both (a) what OS you're on, and (b) whether the remote end has acked all of the data you previously wrote. Given that, it might make sense to have only a single error code covering both cases; for compatibility, this would have to be the existing G_IO_ERROR_BROKEN_PIPE, but we could add an alias for it as G_IO_ERROR_SOCKET_DISCONNECTED... I've pushed an old branch I had lying around to wip/danw/socket-connected. IIRC, the test that it adds passes on Linux, but not on OS X. (That branch also changes the semantics of GSocket:connected in a way that I think we probably don't actually want, but I didn't bother to rebase out that commit.)
Created attachment 291320 [details] [review] gsocket: add G_IO_ERROR_CONNECTION_CLOSED Add G_IO_ERROR_CONNECTION_CLOSED as an alias for G_IO_ERROR_BROKEN_PIPE, and also return it on ECONNRESET. It doesn't really make sense to try to distinguish EPIPE and ECONNRESET at the GLib level, since the exact choice of which error gets returned in what conditions depends on the OS. Given that, we ought to map the two errors to the same value, and since we're already mapping EPIPE to G_IO_ERROR_BROKEN_PIPE, we need to map ECONNRESET to that too. But the existing name doesn't really make sense for sockets, so we add a new name. ========== This is what I'm thinking about committing now. Thoughts?
Review of attachment 291320 [details] [review]: It looks good to me though maybe it would be clearer to do the alias the other way round: enum { ... PIPE = CONNECTION_CLOSED /* for backward compatibility */ } And if does not cause too much churn it would be good to use CONNECTION_CLOSED in all the internal uses of ERROR_PIPE (maybe in a follow up patch?)
> It looks good to me though maybe it would be clearer to do the alias > the other way round: I decided against this. They have to stay in the current order in the docs, and it didn't seem right to have the declaration declare them in the opposite order from the docs. > And if does not cause too much churn it would be good to use > CONNECTION_CLOSED in all the internal uses of ERROR_PIPE (maybe in a > follow up patch?) It turns out that there are no internal uses of BROKEN_PIPE (other than the EPIPE -> G_IO_ERROR_BROKEN_PIPE mapping, which should stay as BROKEN_PIPE), so no, it didn't cause too much churn. :) Attachment 291320 [details] pushed as 967fedc - gsocket: add G_IO_ERROR_CONNECTION_CLOSED
should BROKEN_PIPE be marked as deprecated?
Well, if you're using GUnixOutputStream on a pipe, then BROKEN_PIPE really would be the right error code. Although I guess CONNECTION_CLOSED is a perfectly reasonable name for that condition too... I have no strong opinion either way I guess.