Mon 21 Mar 2016 02:39:16 PM UTC, comment #11:
Moved the issue of blocking during memory failure over to:
bug #47485 (https://savannah.nongnu.org/bugs/index.php?47485)
|
Fri 18 Mar 2016 06:59:16 PM UTC, comment #10:
Right.
> If we want to improve the closure process to have no blocking in the default closure
> behavior, maybe that would be better as another task
Yes. I'd be glad if you could file a bug and add a good description :-)
|
Fri 18 Mar 2016 03:55:54 PM UTC, comment #9:
Simon,
I'm fine with closing this out for now. Both the issues my application ran into are accounted for with changes on master.
If we want to improve the closure process to have no blocking in the default closure behavior, maybe that would be better as another task since this bug was targeted at the socket/netconn leaks
|
Thu 17 Mar 2016 08:13:28 PM UTC, comment #8:
I'm fine with addressing the RST-on-close by translating the err to ERR_OK and allowing the netconn/socket to be cleaned up appropriately. Feel free to move that to a separate bug if desired, thanks!
|
Thu 17 Mar 2016 08:05:41 PM UTC, comment #7:
I've committed the fix for nonblocking sockets, so only the RST-on-close remains. Although that one might deserve its own bug entry...
|
Thu 17 Mar 2016 07:52:05 PM UTC, comment #6:
Reading up on this, you're probably right that lwIP-internal resource shortage should not lead to EWOULDBLOCK on socket layer. I'm OK with changing the netconn API in this respect, too.
I'll change the code to fix this 1st bug. However, there might be
However, deleting the netconn & socket in all 'err != ERR_OK' cases is definitively wrong and can lead to pcb leaks (instead of the netconn/socket leaks you want to fix).
So why don't we solve this 2nd bug by returning ERR_OK when from 'err_tcp()' (which is called on RST) if 'old_state == NETCONN_CLOSE' ? Noone is interested in the RST in this case anyway.
|
Thu 17 Mar 2016 06:56:13 PM UTC, comment #5:
Simon,
Here's the appropriate text. It's located under the section on SO_LINGER (7.5 Generic Socket Options):
"This option specifies how the close function operates for a connection-oriented protocol (e.g., for TCP and SCTP, but not for UDP). By default, close returns immediately, but if there is any data still remaining in the socket send buffer, the system will try to deliver the data to the peer.
The SO_LINGER socket option lets use change this default..."
I agree with you that the ideal behavior would be for LwIP to initiate an asynchronous closure and return caller immediately, even when we can't send a FIN because the send buffer is full.
In my patch, I removed the non-blocking case because my applications are expecting the default close behavior as described by Stevens and won't call close again if it receives EWOULDBLOCK. It was better to block the caller for a couple of milliseconds for an ACK to come in and the send buffer to open up, allowing LwIP to send a FIN and successfully initiate the closure, rather than leak the socket.
My comment #2 describes a case where during the caller being blocked, trying to initiate a closure (current LwIP behavior), a RST comes in and generates an error. This causes the blocking close to return, but with an error, so it leaks the netconn and socket.
I don't think my change to free the socket/netconn is too early because once the non-blocking behavior has been disabled, the call to close either completes successfully or times out. Either case the PCB should be freed by the time we are returning the caller. This is just a work-around and if the final solution we want to leave those intact during the asynchronous closure, that's fine as long as the responsibility has been removed from the application.
These issues are seen with the default close behavior. I have not enabled SO_LINGER support in my port, but care should be taken to make sure we continue adhering to standard SO_LINGER behavior. The winsock documentation has a nice little table describe the closure behaviors (default and SO_LINGER): https://msdn.microsoft.com/en-us/library/windows/desktop/ms737582%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396
|
Thu 18 Feb 2016 07:09:23 PM UTC, comment #4:
I can follow you on Stevens, but he doesn't talk about nonblocking sockets or is he? I don't know if we're doing something wrong here or not.
By making nonblocking sockets behave like blocking sockets, you lose the nonblocking thing, so it might be better to let close return and set a flag in the netconn to dealloc on close success (and yes, always dealloc the socket in close()).
I'm not sure I can follow you on comment #2 though.
The patch in comment #3 seems to dealloc socket/netconn too early without freeing the pcb... or am I wrong?
|
Thu 14 Jan 2016 12:33:20 AM UTC, comment #3:
I'm attaching some work-arounds for this issue (which I'm using in my port) for anyone to use before Simon gets back and we can work through the proper fix.
(file #36021)
|
Thu 17 Dec 2015 07:54:23 PM UTC, comment #2:
I found another case where this is happening. If once in a blocking close, trying to send a FIN, if an RST is received, the TCP error callback is called, returning ERR_RST (ECONNRESET) to the application thread that is blocked. When this happens, the netconn and socket are leaked
The area in netconn_delete() only calls netconn_free() if err equals ERR_OK. The same happens in lwip_close(), free_socket() is only called if err == ERR_OK
This is a slightly different problem than the EWOULDBLOCK as the PCB has indeed been deleted, but then we fail deleting the socket/netconn
I'm not sure if we try to detect if the PCB is gone during error handling in netconn_delete and only delete if so?
Ideally, as outlined below, we would want to delete the netconn/socket in all cases
|
Wed 16 Dec 2015 11:05:31 PM UTC, comment #1:
Here's the Steven's text, apologies for messed up formatting:
|
Wed 16 Dec 2015 11:03:32 PM UTC, original submission:
I've encountered non-standard close behavior that can result in socket and netconn leaks. Long explanation here, please bear with me
My application is using non-blocking sockets and when trying to close the socket, EAGAIN is returned because the snd_buf in the PCB is full and we can't send a FIN
Since the socket is set to non-blocking, the close returns without actually closing the socket. My application, written against standard BSD sockets, isn't checking for EAGAIN as that would not normally be returned from close unless SO_LINGER was set and the timeout elapsed. Even if it was, it doesn't mean the socket is still allocated. With this behavior, non-blocking applications need a wait loop around close until it returns successfully.
I double checked the BSD behavior in my Steven's book (UNIX Network Programming Volume 1, 3rd Edition: The Sockets Networking API) and it documents (section 4.9):
The section on SO_LINGER explains returning EWOULDBLOCK during a SO_LINGER timeout and I can provide that text if it is helpful.
So I see the following deviations:
- close in the default case of SO_LINGER off shouldn't be blocking regardless of blocking/non-blocking socket configuration. The closure happens asynchronously
- close shouldn't ever leave socket resources allocated
- close should only be returning EWOULDBLOCK in the case of a SO_LINGER timeout (or using non-blocking sockets with SO_LINGER configured with a timeout)
I also found this commit in lwip_netconn_do_close_internal, indicating the deviation #1 is known:
I'm thinking why behave differently for nonblocking sockets since we aren't supposed to be blocking for either case? Shorter term fix might just to remove the check netconn_is_nonblocking() and this would make sure close can complete the FIN up till the 20 second timeout.
Longer term, we could change over to having the closure complete asynchronously and having close always deallocate resources. Thoughts?
|