buglwIP - A Lightweight TCP/IP stack - Bugs: bug #31741, lwip_select seems to have...

 
 

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

bug #31741: lwip_select seems to have threading problems

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Thu 02 Dec 2010 11:37:30 AM UTC
   
 
Category:  sockets/netconn Severity:  4 - Important
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Mon 24 Jan 2011 07:30:33 PM UTC, comment #2: 

Yeah, sorry for that mess. Don't know what drove me writing such code :-( That's a really good example why you should not modify the loop variable within the loop...

As to the counter, that's only a 'revision' counter of the select_cb_list: if it has changed, someone has modified the list, meaning we have to re-scan it from the start. That line was simply missing in the part of lwip_select where the list item was removed again.

Should be fixed now.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 11 Jan 2011 02:07:43 PM UTC, comment #1: 

This is now the only known issue preventing the release of 1.4.0.  From the description the solutions sound easy, but I'm not sure I've understood correctly.

For (1) I don't much like the way we're modifying the loop variable within the loop - it makes for difficult to understand (and as shown by this bug!) buggy code.

For (2) I'm not sure which counter is being referred to.

Kieran Mansley <kieranm>
Group Member
Thu 02 Dec 2010 11:37:30 AM UTC, original submission:  

From lwip-users:

As for the crash I mentioned, it occurs in event_callbacks' select
handling, and is not the result of the above-mentioned threading limitation.

It seems to be the result of changing the synchronization mechanism in
1.4.0. lwIP 1.3.2 used a "broad" semaphore to synchronize event_callback
and lwip_select whereas 1.4.0 rc1 uses the critical section and a counter.

The following code in event_callback crashes when dereferencing scb:


        for (scb = select_cb_list; scb; scb = scb->next) {

We have found the following problems:


1. In the event_callback code:


        /* unlock interrupts with each step */

        last_select_cb_ctr = select_cb_ctr;
        SYS_ARCH_UNPROTECT(lev);
        SYS_ARCH_PROTECT(lev);
        if (last_select_cb_ctr != select_cb_ctr) {
            /* someone has changed select_cb_list, restart at the
beginning */
            scb = select_cb_list;

        }


When someone removes the last element in the list (scb == NULL) we
should break out of the loop.


2. In lwip_select, the counter is not updated when an element is removed
from the list.


        /* Take us off the list */
        SYS_ARCH_PROTECT(lev);
        if (select_cb.next != NULL) {
            select_cb.next->prev = select_cb.prev;
        }
        if (select_cb_list == &select_cb) {
            LWIP_ASSERT("select_cb.prev == NULL", select_cb.prev == NULL);
            select_cb_list = select_cb.next;
        } else {
            LWIP_ASSERT("select_cb.prev != NULL", select_cb.prev != NULL);
            select_cb.prev->next = select_cb.next;
        }
        SYS_ARCH_UNPROTECT(lev);

Simon Goldschmidt <goldsimon>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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 kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Submitted the item)
  •  

    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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-01-24 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code