RE : [patch #5919] Create compile switch to removeselectcode

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

RE : [patch #5919] Create compile switch to removeselectcode

Frédéric BERNON
>That's true, and I think that was reported as a problem some time on lwip-users, wasn't it? You can either protect it by using SYS_ARCH_PROTECT() or set conn->recvmbox to SYS_MBOX_NULL and free the mbox after that.

Do you remember the thread? "netconn_recv blocking forever", "netconn_delete and _recv not thread safe", another one? About your solution, if you set the  recvmbox to SYS_MBOX_NULL, it will be difficult to free it after ;) I think it will be simple to do something like this :

if (p == NULL) {
  memp_free(MEMP_NETBUF, buf);
  conn->err = ERR_CLSD; /* or ERR_ABRT, ERR_RST? */
  return NULL;
}

Like this, the application know it have to close the socket, and no other operation (send/recv) can be done, because we have test like "if (conn->err != ERR_OK)"...
 
====================================
Frédéric BERNON
HYMATOM SA
Chef de projet informatique
Microsoft Certified Professional
Tél. : +33 (0)4-67-87-61-10
Fax. : +33 (0)4-67-70-85-44
Email : [hidden email]
Web Site : http://www.hymatom.fr 
====================================
P Avant d'imprimer, penser à l'environnement
 


-----Message d'origine-----
De : lwip-devel-bounces+frederic.bernon=[hidden email] [mailto:lwip-devel-bounces+frederic.bernon=[hidden email]] De la part de Goldschmidt Simon
Envoyé : mercredi 23 mai 2007 08:46
À : lwip-devel
Objet : RE: [lwip-devel] [patch #5919] Create compile switch to removeselectcode



> Follow-up Comment #11, patch #5919 (project lwip):
>
> I think to extend LWIP_SO_RCVTIMEO to TCP connections. It
> will be the same code, or something near...

What does that have to do with 'Create compile switch to
remove select code'?? :-)

>
> But, I don't like this part of code in netconn_recv:
>
>     /* If we are closed, we indicate that we no longer wish to receive
>        data by setting conn->recvmbox to SYS_MBOX_NULL. */
>     if (p == NULL) {
>       memp_free(MEMP_NETBUF, buf);
>       sys_mbox_free(conn->recvmbox);
>       conn->recvmbox = SYS_MBOX_NULL;
>       return NULL;
>     }
>
> I think it's not very thread-safe...
That's true, and I think that was reported as a problem some time on lwip-users, wasn't it? You can either protect it by using SYS_ARCH_PROTECT() or set
conn->recvmbox to SYS_MBOX_NULL and free the mbox after that.

Either way, in api_msg.c in the recv_*() functions, there is
a long way between the test of conn->recvmbox and the post to it, so the post might fail since posting to SYS_MBOX_NULL. And all that while conn->recv_avail has been updated and
conn->callback() has been called...

This leads to me the question: is it really necessary to delete the mbox? If TCP told us we're closed, it shouldn't send us more data anyway. And even if it would, we would receive it in netconn_delete() (with the little overhead that tcpip_thread would post it, but that should not happen anyway).


Simon


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

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

=?iso-8859-1?Q?Fr=E9d=E9ric_BERNON=2Evcf?= (810 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: RE : [patch #5919] Create compile switch toremoveselectcode

Goldschmidt Simon

 
> Do you remember the thread? "netconn_recv blocking forever",
> "netconn_delete and _recv not thread safe", another one?
> About your solution, if you set the  recvmbox to
> SYS_MBOX_NULL, it will be difficult to free it after ;) I
> think it will be simple to do something like this :

I would, of course, save the pointer in a temporary variable!


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

RE: RE : [patch #5919] Create compile switch toremoveselectcode

Goldschmidt Simon
In reply to this post by Frédéric BERNON
> I
> think it will be simple to do something like this :
>
> if (p == NULL) {
>   memp_free(MEMP_NETBUF, buf);
>   conn->err = ERR_CLSD; /* or ERR_ABRT, ERR_RST? */
>   return NULL;
> }
>
> Like this, the application know it have to close the socket,
> and no other operation (send/recv) can be done, because we
> have test like "if (conn->err != ERR_OK)"...

That's a good idea!


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