Re: [libmicrohttpd] MHD_quiesce_daemon() question

[silvioprog]

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 <address@hidden> 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