View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0005825 | GNUnet | transport service | public | 2019-08-10 21:41 | 2023-06-02 19:16 |
Reporter | schanzen | Assigned To | schanzen | ||
Priority | normal | Severity | minor | Reproducibility | have not tried |
Status | closed | Resolution | suspended | ||
Summary | 0005825: setsockopt SO_NOSIGPIPE can fail after socket accept | ||||
Description | I had to modify the network.c code a bit to find that setting the sockopt NOSIGPIPE after accept() often fails in trasport. This happens only in macos. After investigating it seems like this happens when the remote peer closes the socket before we set the option. The behaviour is thus correct but spams the logs with the below warnings (more general). The questions is if we can ignore the error at least for transport? At least accept it and log it as DEBUG? Usually, the handling of this event (error in sockopt after accept) is to try and reconnect. | ||||
Additional Information | Aug 10 21:24:30-522872 transport-34891 ERROR Assertion failed at network.c:452. Aug 10 21:24:30-526849 transport-34891 WARNING `accept' failed at service.c:832 with error: Invalid argument | ||||
Tags | No tags attached. | ||||
|
I would disable the warning for the *specific* errno that tells you that the other side already closed the connection -- and then close() the connection on our end and return NULL as well. Additionally, on platforms where this is possible, we should use accept4() to set the socket options immediately upon accepting, instead of burning additional syscalls later. |
|
Well, the problem is specifically that the errno in this case is "Invalid argument" (EINVAL), because we are calling it on a socket which was closed by the remote peer. between our accept and setockopt. |
|
Eh, based on my understanding that would definitively not result in an EINVAL. Looks to me more like some *other* argument to setsockopt() is invalid on that platform (so the FD is OK, but value/sizeof/IP option) are invalid. For example, we use IPPROTO_TCP, which would be bad for a UNIX/UDP socket. Do we 'accept' on UNIX domain sockets? Then setsockopt() of socket_set_nodelay() would fail with EINVAL because the address family of the socket does not match the setsockopt() call. It must be something like this, I don't believe the half-closed story for a second. |
|
Unlikely, because the error does not happen _always_, but only when the remote peer closed the connection prematurely. And in that case setting the sockopt is invalid. Hence, EINVAL. I have been digging around that quite a bit and it seems as if this is not against the spec, but only really happens this way on macos. |
|
Well, I guess it can't hurt to add logic to react to a failed setsockopt() with a close(). We know setsockopt() should virtually always succeed, so if it does not,logging + close() seems to be a sensible reaction. |
|
I do not really see any negative impact on the user experience itself beyond odd behaviour when peers are already crashing locally. So suspending. |
Date Modified | Username | Field | Change |
---|---|---|---|
2019-08-10 21:41 | schanzen | New Issue | |
2020-10-29 11:23 | schanzen | Assigned To | => schanzen |
2020-10-29 11:23 | schanzen | Status | new => feedback |
2020-10-29 11:23 | schanzen | Assigned To | schanzen => Christian Grothoff |
2020-10-29 11:29 | Christian Grothoff | Note Added: 0017058 | |
2020-10-29 11:29 | Christian Grothoff | Assigned To | Christian Grothoff => schanzen |
2020-10-29 11:30 | Christian Grothoff | Status | feedback => assigned |
2020-11-02 09:12 | schanzen | Note Added: 0017080 | |
2020-11-08 18:09 | Christian Grothoff | Note Added: 0017111 | |
2020-11-11 10:49 | schanzen | Note Added: 0017119 | |
2020-12-23 12:57 | Christian Grothoff | Note Added: 0017229 | |
2022-09-28 15:40 | schanzen | Status | assigned => resolved |
2022-09-28 15:40 | schanzen | Resolution | open => suspended |
2022-09-28 15:40 | schanzen | Note Added: 0019194 | |
2023-06-02 19:16 | schanzen | Status | resolved => closed |