Re: [libmicrohttpd] More useful MHD_quiesce_daemon in threaded mode

[Christian Grothoff]

>>> I think the patch would be simple -- you could just kill the threads
>>> in the MHD_quiesce_daemon. Or, if you do not like that, you could
>>> make the MHD_select_thread(s) exit not only if daemon->shutdown, but
>>> also if daemon->socket_fd == MHD_INVALID_SOCKET, and use wpipe at the
>>> end of MHD_quiesce_daemon to wake them up and pthread_join them.
>>> (Opened wpipe would be needed, but there are already some (not really
>>> needed) tests for opened wpipe in MHD_USE_SELECT_INTERNALLY at the
>>> beginning of MHD_quiesce_daemon.)
>>
>> The semantics you propose you can already get by calling 'dup()' on the
>> listen FD (which you can get from MHD_get_daemon_info()) and then
>> calling MHD_daemon_stop().  But that brutally closes ongoing
>> connections, which may not be ideal.
> 
> What I tried to suggest was to remove the socket_fd from all
> selects/polls when MHD_quiesce_daemon is called. That could be done (for
> example as described) and would not break any active connections (the
> listening socket and the client sockets are independent). But the only
> gain would be that the socket_fd could be closed before MHD_stop_daemon,
> which is probably not worth it.
> 
> Nevertheless, this means that there is an unhandled special case.
> Consider MHD_USE_THREAD_PER_CONNECTION (either with or without
> MHD_USE_POLL). Then wpipe is not created. After MHD_quiesce_daemon,
> socket_fd = -1. Then, after the current select/poll in the
> MHD_select_thread exits, there will be no fds to wait for (the socket_fd
> is -1 and wpipe was not created), so the MHD_select_thread will be
> busy-waiting for daemon->shutdown. Therefore, another condition should
> be added to the beginning of MHD_quiesce_daemon:
> 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').

Happy hacking!

Christian

signature.asc
Description: OpenPGP digital signature