Race condition with lwip_select (and potential fix)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Race condition with lwip_select (and potential fix)

Chris Deighton
Hi all,

I've been noticing occasional crashes within lwip when using select(). I think
I've tracked the problem down to a race condition hidden in the interaction
between lwip_select() and event_callback() (both in sockets.c).

The failure case is as follows:

User Thread: Calls lwip_select():
   - Creates a select_cb on the stack.
   - Creates select_cb.sem.
   - Takes selectsem.
     - Inserts the select_cb into select_cb_list.
   - Releases selectsem.
   - Waits for select_cb.sem to be signalled (or timeout to expire).

LwIP Thread: Data arrives, so calls event_callback():
   - Takes selectsem.
     - select_cb_list is examined and scb stores a pointer to the select_cb.
   - Releases selectsem.

User Thread: Timeout expires, so resumes execution of lwip_select():
   - Takes selectsem.
     - Removes the select_cb from select_cb_list.
   - Releases selectsem.
   - Frees select_cb.sem.

LwIP Thread: Resumes execution of event_callback():
   - Tries to signal scb->sem, but select_cb and/or sem no longer exist.

Based on the above analysis, the fix would appear to be to swap the order in
which event_callback() signals the two semaphores. I've tried this and so far
it appears to work. I'd appreciate any thoughts from other LwIP users though!

Regards,

Chris.




_______________________________________________
lwip-users mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/lwip-users
Reply | Threaded
Open this post in threaded view
|

Re: Race condition with lwip_select (and potential fix)

Jonathan Larmour
Chris Deighton wrote:
>
> Based on the above analysis, the fix would appear to be to swap the
> order in which event_callback() signals the two semaphores. I've tried
> this and so far it appears to work. I'd appreciate any thoughts from
> other LwIP users though!

Makes sense to me, for what it's worth.

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
------["The best things in life aren't things."]------      Opinions==mine


_______________________________________________
lwip-users mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/lwip-users
Reply | Threaded
Open this post in threaded view
|

Re: Race condition with lwip_select (and potential fix)

Jonathan Larmour
Jonathan Larmour wrote:
> Chris Deighton wrote:
>>
>> Based on the above analysis, the fix would appear to be to swap the
>> order in which event_callback() signals the two semaphores. I've tried
>> this and so far it appears to work. I'd appreciate any thoughts from
>> other LwIP users though!
>
> Makes sense to me, for what it's worth.

Doh, I was going to add... once you're happy with it, you could submit a
patch to http://savannah.nongnu.org/patch/?group=lwip then hopefully a
developer will apply.

Jifl
--
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
------["The best things in life aren't things."]------      Opinions==mine


_______________________________________________
lwip-users mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/lwip-users