Re: [libmicrohttpd] More useful MHD_quiesce_daemon in threaded mode

[Christian Grothoff]

Got it, yes, you're right, fixed in SVN 34385. -Christian

On 10/28/2014 12:47 AM, Milan Straka wrote:
>>> current:
>>>
>>>   MHD_quiesce_daemon (struct MHD_Daemon *daemon)
>>>   {     
>>>     unsigned int i;
>>>     int ret;
>>>
>>>     ret = daemon->socket_fd;
>>>     if (-1 == ret)
>>>       return -1;
>>>     if ( (-1 == daemon->wpipe[1]) &&
>>>          (0 != (daemon->options & MHD_USE_SELECT_INTERNALLY)) )
>>>     {
>>> #if HAVE_MESSAGES
>>>       MHD_DLOG (daemon,
>>>                 "Using MHD_quiesce_daemon in this mode requires 
>>> MHD_USE_PIPE_FOR_SHUTDOWN\n");
>>> #endif
>>>       return -1;
>>>     }
>>>
>>> new:
>>>
>>>     if ( (-1 == daemon->wpipe[1]) &&
>>>          (0 != (daemon->options & (MHD_USE_SELECT_INTERNALLY | 
>>> MHD_USE_THREAD_PER_CONNECTION )) ))
>>>
>>> Did I get it right?
>>
>> I don't think so.  Note that the 'socket_fd' in the thread's
>> select()/poll() call is not the (closed) listen socket, but the thread's
>> TCP connection to the client.  So the thread can still be terminated by
>> calling shutdown() on that TCP connection socket.
>> (The code is a bit confusing here, as both structs have a member called
>> 'socket_fd').
> 
> (I am sorry I cannot make myself clear enough.)
> 
> I still think what I wrote holds. I am talking about the thread
> executing MHD_select_thread, i.e., the one created in MHD_start_daemon_va.
> It gets the daemon structure as parameter and accesses daemon->socket_fd.
> I believe you are talking about threads executing MHD_handle_connection,
> created in internal_add_connection.
> 
> The problem is that the listening thread might run out of FDs to wait
> for. In the MHD_USE_THREAD_PER_CONNECTION case, the main listening
> thread only selects/polls on two FDs -- daemon->socket_fd, and
> daemon->wpipe[0]. And when daemon->socket_fd is closed and
> daemon->wpipe[0] does not exist, problem occurs.
> 
> The same issue could happen when there is a thread_pool, but that is
> taken care of in MHD_quiesce_daemon -- in this case, if there is no
> wpipe, MHD_quiesce_daemon fails. What I am suggesting that this test
> should also include the MHD_USE_THREAD_PER_CONNECTION case.
> 
> Cheers,
> Milan Straka
>

0x48426C7E.asc
Description: application/pgp-keys

signature.asc
Description: OpenPGP digital signature