Thanks for replying Evgeny, you and Christian always saving my day. :-)

I read and re-read your and Christian answer, and finally I found a
possible way to never lock the server at shutdown: adding an option to exit
the server after configurable attempts. It can solve the timeout problem
too, because I can't ensure that the programmer have configured it (anyway
my library will assume 15 seconds by default). So, the code below applies
your tips and implements the attempts approach:

bool bf_httpsrv_shutdown(struct bf_httpsrv *srv) {
    MHD_socket fd;
    uint8_t shutdown_attempts; /* Max 256. */
    if (srv && srv->listening) {
        fd = MHD_quiesce_daemon(srv->mhd);
        if (fd != MHD_INVALID_SOCKET)
            close(fd);
        else
            _BF_LOG("Server quiesce failed.\n");
        if (srv->forced_shutdown) {
            shutdown_attempts = 1;
            while (MHD_get_daemon_info(srv->mhd,
MHD_DAEMON_INFO_CURRENT_CONNECTIONS)->num_connections > 0) {
                if (shutdown_attempts >= srv->shutdown_attempts) { /*
Default srv->shutdown_attempts is 10. */
                    _BF_LOG("Forced server shutdown.\n");
                    exit(EINTR);
                }
                shutdown_attempts++;
                sleep(1);
            }
        }
        MHD_stop_daemon(srv->mhd);
        srv->listening = false;
        return true;
    }
    return false;
}


Unfortunately if exit was called the MHD_stop_daemon() will be not called,
probably raising some memory leak, but current I have no idea how it could
be solved.

Feel totally free to point improvements about this new version. :-)

On Sun, Mar 26, 2017 at 5:36 PM, Evgeny Grin <k...@yandex.ru> wrote:

> On 26.03.2017 8:33, silvioprog wrote:
> > I found the following related message:
> >
> > https://lists.gnu.org/archive/html/libmicrohttpd/2014-09/msg00012.html
> >
> > I've used a similar logic, but with item X below, because I need to wait
> > the client processing:
> >
> > 1) MHD_quiesce_daemon()
> > *X) while (info.num_connections > 0) sleep(0.5s) # pseudo code*
> > 2) MHD_stop_daemon()
> > 3) close()
> >
> > Real implementation:
> >
> > bool bf_httpsrv_shutdown(struct bf_httpsrv *srv, bool force) {
> >     MHD_socket fd;
> >     if (srv && srv->listening) {
> >         fd = MHD_quiesce_daemon(srv->mhd);
> >         if (!force)
> >             while (MHD_get_daemon_info(srv->mhd,
> MHD_DAEMON_INFO_CURRENT_CONNECTIONS)->num_connections > 0)
> >                 usleep(1000 * 500); //TODO: allow to use external
> callback
> >         MHD_stop_daemon(srv->mhd);
> >         if (fd != MHD_INVALID_SOCKET)
> >             close(fd);
> >         srv->listening = false;
> >         return true;
> >     }
> >     return false;
> > }
> >
> >
> > Calling it with bf_httpsrv_shutdown(srv, false) the server stops waiting
> > for clients processing.
> >
> > So, what do you think about the logic above? Should it be improved?!
> >
> > Thanks in advance for any suggestions!
>
> If you don't check returned value from MHD_quiesce_daemon(), you may
> later found that you didn't quiesced daemon, so move check right after
> MHD_quiesce_daemon() and added error handling.
> If you didn't set connection timeout, connection may live indefinitely.
> Moreover, even with connection timeout, clients may continue processing
> on same HTTP 1.1 connections with new requests indefinitely.
> Furthermore, even with HTTP 1.0 and connection timeout hypothetical
> client may read answer very slow with results in very long unpredictable
> closure of connection.
> So: yes, you code will work but may result in very long (hours, for
> example) or even indefinitely long daemon shutdown.
>
> --
> Best Wishes,
> Evgeny Grin
>

-- 
Silvio Clécio

Reply via email to