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 <[email protected]> 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
