[lwip-users] netconn_delete and _recv not thread safe

[Jonathan Larmour]

I have most of the lwip stack running in a "tcpip" thread, and stack users 
in various threads synchronise with it using the netconn* API. But there is 
a problem with netconn_delete and netconn_recv when they delete mboxes. A 
packet, or a connection to a listening socket, may arrive in the gap 
between removing mbox items, and deleting the mboxes. For example:

  /* Drain the recvmbox. */
  if (conn->recvmbox != SYS_MBOX_NULL) {
    while (sys_arch_mbox_fetch(conn->recvmbox, &mem, 1) != SYS_ARCH_TIMEOUT) {
      if (conn->type == NETCONN_TCP) {
        if(mem != NULL)
          pbuf_free((struct pbuf *)mem);
      } else {
        netbuf_delete((struct netbuf *)mem);
      }
    }

*** PACKET CAN ARRIVE HERE ***
    sys_mbox_free(conn->recvmbox);
    conn->recvmbox = SYS_MBOX_NULL;
  }

In that case a sys_mbox_free can be called on a non-empty mbox. This may 
either leak a pbuf, or cause an assertion in the OS from the underlying 
mbox implementation. I've been thinking about how to fix this. Either new 
sys_arch primitives could be added. Or, better, we could try to use 
SYS_LIGHTWEIGHT_PROT, but this seems a bit dubious as we need to protect 
from before the sys_arch_mbox_fetch, until the conn->recvmbox = SYS_MBOX_NULL.

This may mean calling sys_arch_mbox_fetch, which has a timeout of 1, with 
the protection (of whatever form) on, which is a bad idea.

It may be possible to solve this if it is ok to assign mboxes:
    SYS_ARCH_PROTECT(old_level);
    tmp_mbox = conn->recvmbox;
    conn->recvmbox = SYS_MBOX_NULL;
    SYS_ARCH_UNPROTECT(old_level);

Is assuming mboxes are assignable a valid approach? Any other thoughts? I 
can't think of any other alternative than adding to the sys_arch API.

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