Re: [libmicrohttpd] More useful MHD_quiesce_daemon in threaded mode

[Milan Straka]

Hi Christian,

> -----Original message-----
> From: Christian Grothoff <address@hidden>
> Sent: 27 Oct 2014, 18:22
>
> On 10/27/2014 05:13 PM, Milan Straka wrote:
> > Hi all,
> > 
> > I am from Charles University in Prague and I am using libmicrohttpd as
> > HTTP server in several RESTful services provided by our department
> > (linguistics, for example http://lindat.mff.cuni.cz/services/morphodita/).
> > I use MHD_USE_THREAD_PER_CONNECTION | MHD_USE_POLL and need to continue
> > using some threaded mode (the services response takes quite some time to
> > compute).
> 
> Sure, makes sense.
> 
> > I am now facing an issue how to upgrade the running service seamlessly.
> > I need to stop listening on the socket, wait reasonable time for the
> > requests being served to finish, and stop.
> > 
> > Nevertheless, as stated in the documentation, the MHD_select_thread(s)
> > still has the listening socket after MHD_quiesce_daemon and can accept
> > one incoming connection at any time.
> 
> I don't quite see the documentation saying that.  The documentation says:
> 
> @deftypefun int MHD_quiesce_daemon (struct MHD_Daemon *daemon)
> @cindex quiesce
> Stop accepting connections from the listening socket.  Allows clients
> to continue processing, but stops accepting new connections.
> 
> So after MHD_quiesce_daemon, MHD will no longer accept connections on
> that socket. You're not supposed to 'close()' on it in multi-threaded
> mode because the socket _may_ be still in a 'select()' set and if you
> close() it we get some semi-defined behavior, but that does not mean
> that MHD might still call 'accept()' on the socket.

Oh, my error. When inspecting MHD_poll_listen_socket, I saw it calling
MHD_accept_connection, even though MHD_run_from_select called it only
when socket_fd is not -1, and got an impression that the connection
could be accepted (which is nonsense, since on what socket would the
accept be called, with socket_fd == -1).

Therefore, MHD_quiesce_daemon can be used as I want.

> It is also safe to pass the listen socket to another process or another
> MHD instance to start accepting again.
> 
> > I believe it would be better to stop the MHD_select_thread(s) in the
> > MHD_quesce_daemon. That would allow to return the listening FD which
> > would no longer be referenced anywhere in the daemon, making sure that
> > no more connections are accepted and allowing user to manipulate the FD
> > as they see fit. This behaviour is actually the one I was expecting when
> > discovering MHD_quiesce_daemon.
> 
> Well, the issue is that then the active connections would break. So
> instead, we allow your process to pass on listening activity AND still
> cleanly finish existing requests.
> 
> > 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?

Cheers,
Milan Straka