lwIP - A Lightweight TCP/IP stack - Bugs: bug #46696, accepts_pending not decreased when...
You are not allowed to post comments on this tracker with your current authentication level.
bug #46696: accepts_pending not decreased when TCP_EVENT_ACCEPT returns error
Submitter: | Joel Cunningham <jcunningham> | ||
Submitted: | Wed 16 Dec 2015 03:37:22 PM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Faulty Behaviour | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | git head |
Jump to the original submission
Fri 25 Mar 2016 04:13:17 PM UTC, comment #22: |
Simon Goldschmidt <goldsimon> |
Fri 25 Mar 2016 09:41:59 AM UTC, comment #21:
|
Simon Goldschmidt <goldsimon> |
Thu 24 Mar 2016 11:02:41 PM UTC, comment #20: Ambroz,
|
Joel Cunningham <jcunningham> |
Thu 24 Mar 2016 05:14:53 PM UTC, comment #19: Hey,
|
Ambroz Bizjak <abizjak> |
Thu 24 Mar 2016 03:12:47 PM UTC, comment #18: I also took a look at the changes and agree the inconsistent state situations seems to come about because the upper layers are currently required to perform a matching pair of tcp_backlog_delayed() and tcp_backlog_accepted() calls. If the child PCB is freed internally before this happens and is in state ESTABLISHED, we have a leak in the listener.
|
Joel Cunningham <jcunningham> |
Thu 24 Mar 2016 07:33:29 AM UTC, comment #17: The internal backlog should be OK I think.
|
Simon Goldschmidt <goldsimon> |
Wed 23 Mar 2016 09:47:40 PM UTC, comment #16: Ah, forget about the part regarding tcp_abandon(), I missed that it calls TCP_PCB_REMOVE_ACTIVE-->tcp_pcb_remove-->tcp_pcb_purge. |
Ambroz Bizjak <abizjak> |
Wed 23 Mar 2016 09:42:19 PM UTC, comment #15: Hello Simon,
|
Ambroz Bizjak <abizjak> |
Wed 23 Mar 2016 07:45:20 AM UTC, comment #14: Fixed by changing accept and backlog: instead of having an accept callback in the cnnection pcb, there is now a pointer to the listening pcb. That way, accepts_pending can be decreased very fast. This is now done internally in the stack ('tcp_accepted()' got useless now).
|
Simon Goldschmidt <goldsimon> |
Sun 06 Mar 2016 05:18:06 PM UTC, comment #13: I'd be glad to get a better working version of the "accepts_pending" implementation. The current way of letting the user application keeping track of the corresponding listen pcb is really bad.
|
Simon Goldschmidt <goldsimon> |
Thu 21 Jan 2016 05:59:07 PM UTC, comment #12: Also netconn should be releasing the reference in the error paths in accept_function(). Assuming that the lwip core is not changed with respect to releasing the reference itself. |
Ambroz Bizjak <abizjak> |
Thu 21 Jan 2016 05:56:51 PM UTC, comment #11: Hey Joel,
|
Ambroz Bizjak <abizjak> |
Thu 21 Jan 2016 04:58:43 PM UTC, comment #10: Ambroz,
|
Joel Cunningham <jcunningham> |
Tue 19 Jan 2016 06:00:02 PM UTC, comment #9: Also, I wouldn't complicate the situation by interpreting errors from the accept callback differently as long as the accepts_pending reference is concerned. The accept callback should assume ownership of a single reference regardless of what error code it returns. |
Ambroz Bizjak <abizjak> |
Tue 19 Jan 2016 05:55:06 PM UTC, comment #8: I think it doesn't matter if you manipulate the backlog or accepts_pending. If you want to have separate control over the backlog, incrementing/decrementing the accepts_pending instead of decrementing/incrementing the backlog makes sense.
|
Ambroz Bizjak <abizjak> |
Tue 19 Jan 2016 05:30:26 PM UTC, comment #7: Ambroz,
|
Joel Cunningham <jcunningham> |
Mon 18 Jan 2016 09:45:38 PM UTC, comment #6: Note you don't even need to change core lwIP code to do this, just cast the tcp_pcb* into tcp_pcb_listen* and have fun with the ->backlog.
|
Ambroz Bizjak <abizjak> |
Mon 18 Jan 2016 09:37:42 PM UTC, comment #5: Hey Joel,
|
Ambroz Bizjak <abizjak> |
Mon 18 Jan 2016 05:19:38 PM UTC, comment #4: Ambroz,
|
Joel Cunningham <jcunningham> |
Sun 17 Jan 2016 05:58:03 PM UTC, comment #3: Hey,
|
Ambroz Bizjak <abizjak> |
Wed 16 Dec 2015 09:39:19 PM UTC, comment #2: Thinking about this more, the tcp_accept_null() function also has the same problem (actually any accept callback that calls tcp_abort and returns ERR_ABRT). It calls tcp_abort(), but since the connection is in ESTABLISHED state, accepts_pending won't be decremented
|
Joel Cunningham <jcunningham> |
Wed 16 Dec 2015 05:44:12 PM UTC, comment #1: Attached is a patch for solution #2 |
Joel Cunningham <jcunningham> |
Wed 16 Dec 2015 03:37:22 PM UTC, original submission:
I'm running LwIP master branch and in my product (NO_SYS = 0, sockets/netconn enabled), I've encountered the following behavior:
|
Joel Cunningham <jcunningham> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 11 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2016-03-25 | goldsimon | Status | In Progress | Fixed | |
Open/Closed | Open | Closed | |||
2016-03-25 | goldsimon | Status | Fixed | In Progress | |
2016-03-24 | goldsimon | Open/Closed | Closed | Open | |
2016-03-23 | goldsimon | Status | In Progress | Fixed | |
Open/Closed | Open | Closed | |||
2016-03-06 | goldsimon | Status | None | In Progress | |
Assigned to | None | goldsimon | |||
Planned Release | 1.5.0 | ||||
2016-01-18 | abizjak | Attached File | - | Added lwip-backlog.patch, #36087 | |
2015-12-16 | jcunningham | Attached File | - | Added 0001-Fix-for-accepts_pending-leak-on-failed-TCP_EVENT_ACC.patch, #35765 |
Powered by Savane 3.13-3230.
Corresponding source code
Fixed by implementing the flag and always calling tcp_backlog_accepted() from tcp_pcb_purge().