[lwip-devel] [bug #23240] recv_udp increases counters for available receives before netbuf is actually posted

[Jonathan Larmour]

Follow-up Comment #2, bug #23240 (project lwip):

I looked at it, but not closely enough to be sure about whether it's safe to
change the order of the trypost, recv_avail increment, and event callback.
Certainly I believe the counters are not the crucial bit in the code affected
- the API_EVENT call is.

I'm not sure the proposed fix would be reliable. Consider the following
example scenario with my belief (not proved empirically) of what can happen:

1) TCPIP thread does successful trypost to recvmbox
2) Application thread pre-empts, calls lwip_read, which calls lwip_recvfrom
which calls netconn_recv
3) netconn_recv fetches data from recvmbox
4) netconn_recv decrements conn->recv_avail, which being u16_t wraps round to
65535
4) netconn_recv calls API_EVENT(conn, NETCONN_EVT_RCVMINUS, len);
5) That calls event_callback(), and sock->rcvevent is a u16_t and now wraps
round to 65535.
6) app calls lwip_select which calls lwip_selscan which tests rcvevent which,
being 65535, it considers ready for reading even though it isn't.

I've considered doing a NETCONN_EVT_RCVPLUS event and then follow it
immediately with an NETCONN_EVT_RCVMINUS on error, but select will still have
woken the thread. And if the thread is higher priority than the tcpip thread
it may run and be attempting a read before we were able to do the RCVMINUS.

Perhaps it would be sufficient to change rcvevent (and recv_avail for good
measure) to signed quantities and check where they are tested to treat
negative like 0 when deciding if there's enough space. Then the suggested fix
may work.

BTW, this also affects recv_raw.

Incidentally, I'm not sure about the LWIP_DEBUGF addition. If that's to be
there are at all then it should be API_MSG_DEBUG.




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?23240>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/