buglwIP - A Lightweight TCP/IP stack - Bugs: bug #47485, tcp_close() should not fail on...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #47485: tcp_close() should not fail on memory error

Submitted by:  Joel Cunningham <jcunningham>
Submitted on:  Mon 21 Mar 2016 02:37:30 PM UTC  
 
Category:  TCP Severity:  3 - Normal
Item Group:  Change Request Status:  Fixed
Privacy:  Public Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Fri 10 Feb 2017 03:14:03 PM UTC, comment #4: 

  /* - states which free the pcb are handled here,
     - states which send FIN and change state are handled in  tcp_close_shutdown_impl() */

Small typo here, function name should be "tcp_close_shutdown_fin()"

Glad to see this fix :)

Joel Cunningham <jcunningham>
Project Member
Thu 15 Dec 2016 09:25:38 PM UTC, comment #3: 

The problem I still see here is that tcp_send_fin() can fail in tcp_enqueue_flags() due to segment/pbuf allocation failure.

For the rest of the stack (as well as for raw API applications), it would be easier if this failure would lead to retrying FIN instead of returning an error. If we could ensure FIN was enqueued at some time, tcp_close() could just return ERR_OK, which would greatly help to fix this bug.

--> changed summary & category to reflect that this is a tcp core issue

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 21 Jul 2016 04:36:33 PM UTC, comment #2: 

The change for bug #48543 removes the requirement of having space in the send buffer to send a FIN.  That eliminates the particular memory error case I had encountered when trying to close a non-blocking socket with a full send buffer.

This issue is still present if there are other memory allocation failures that could occur during sending of the FIN (which we would need to block until they could be resolved)

Joel Cunningham <jcunningham>
Project Member
Mon 20 Jun 2016 02:35:37 PM UTC, comment #1: 

Simon,

What do you think about targeting this for 2.0.1 since I think the proper fix will will requires more changes than a simple one-liner?

Joel Cunningham <jcunningham>
Project Member
Mon 21 Mar 2016 02:37:30 PM UTC, original submission:  

lwip_netconn_do_close_internal() will keep the calling thread (which issued close()) blocked if a memory error is encountered.

This has been observed in the case where an application tries to close a TCP socket when the send buffer is full.  When a socket's TCP buffer is full, we can't send a FIN packet (due to logic inside the TCP core).

The calling thread will stay blocked until LWIP_TCP_CLOSE_TIMEOUT_MS_DEFAULT expires or we receive an ACK and space opens up in the send buffer, allowing use to send a FIN. 

Here's the description of default close behavior in BSD sockets.  Taken from Steven'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."

In order to conform to the BSD behavior, we need a mechanism in LwIP to asynchronously initiate the closure again after encountering a memory failure in lwip_netconn_do_close_internal().  The calling thread would be unblocked in all execution paths and we could trigger the closure again in sent_tcp upon receiving an ACK.  Other memory failures may need to be addressed appropriately.  The LWIP_TCP_CLOSE_TIMEOUT_MS_DEFAULT timeout would also apply if the memory condition never resolves itself

Joel Cunningham <jcunningham>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by jcunningham (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-10 goldsimon StatusNone => Fixed
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2016-12-15 goldsimon Categorysockets/netconn => TCP
        Summarylwip_netconn_do_close_internal blocks caller during memory error => tcp_close() should not fail on memory error

    Back to the top


    Powered by Savane 3.5