buglwIP - A Lightweight TCP/IP stack - Bugs: bug #28062, Data received directly after...

 
 

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

bug #28062: Data received directly after accepting does not wake up select

Submitted by:  Simon Goldschmidt <goldsimon>
Submitted on:  Fri 20 Nov 2009 03:32:25 PM UTC  
 
Category: sockets/netconnSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

Fri 20 Nov 2009 04:16:38 PM UTC, comment #2:

I just tested it and can confirm the bug is there and fixed by the proposed patch.

Checked in, thanks for finding this.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 20 Nov 2009 03:33:50 PM UTC, comment #1:

In other words, in event_callback(), add the line 's = conn->socket;' before the second 'sys_sem_signal(socksem);'.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 20 Nov 2009 03:32:25 PM UTC, original submission:

As reported by Albert Bartel on lwip-devel:

Finally we found the bug that was responsible for the communication hang for 60sec after a single ack.

It could be easy reproduced with fast and short requests, much more often with vista than with xp (because tcp/ip with vista is faster than with xp).

The request was received, correctly processed and a NETCONN_EVT_RCVPLUS should be sent by calling API_EVENT() but because of speed problems the conn->socket (that is prior also stored in the local variable s) is invalid after the first try.

After the sys_sem_wait(socksem) it is checked again. If it is still invalid, there is a return.

But if we now got a valid conn->socket we get the socket with sock = get_socket(s), but s is still invalid.

Because s is used later, s should be set to conn->socket after we get a valid conn->socket. (see comment)

s = conn->socket;

(1) if (s < 0) {

/* Data comes in right away after an accept, even though

* the server task might not have created a new socket yet.

* Just count down (or up) if that's the case and we

* will use the data later. Note that only receive events

* can happen before the new socket is set up. */

sys_sem_wait(socksem);

(2) if (conn->socket < 0) {

if (evt == NETCONN_EVT_RCVPLUS) {

conn->socket--;

}

sys_sem_signal(socksem);

return;

}

// set s to conn->socket because now conn->socket s valid

// and s (old conn->socket) is invalid

(3) s = conn->socket;

sys_sem_signal(socksem);

}

sock = get_socket(s);

if (!sock) {

return;

}

In our case it was frequently that the first check (1) failed because s (conn->socket) was -1.

So we get into the first if (1). There we wait for the semaphore to be signalled and after that there is a second check (2). If this check fails, we get into the next if (2) and there we return in every case.

Otherwise, if the second check (2) succeeds, we signal the semaphore and call sock = get_socket(s), but s is still -1. So get_socket(s) fails, we return and the socket is not signalled, that there was data received.

If we get into the second if (2), the conn->socket is still invalid and the return is correct, of course. But if we don’t get into the second if (2), the conn->socket is valid, isn’t it?

And because s is used later on, we have got to set s to the now valid
conn->socket.

In our case, when we got into (1) but not into (2) we never receive a signal that data was received, so lwip_selscan in lwip_select never returns > 0 because ->rcvevent is not incremented. And because the received data was the HTTTP GET, no more data follows, the data is not processed and we get no answer until the connection is closed manually (then the FIN,ACK increments the ->rcvevent and we get the data after the connection closed).

So in our case it works, every connection is processed correctly. That’s why I think, that (3) is correct and should be inserted.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 20 Nov 2009 04:16:38 PM UTCgoldsimonOpen/ClosedOpen=>Closed
      StatusNone=>Fixed

    Back to the top


    Powered by Savane 3.1-cleanup1