Dude, thanks for fixing it, I'm going to study the changes and apply it
here. :-)
I've got a situation that locks the shutdown even using this fix, but I
need to create a small project simulating the problem, I'm going to do it
now ...
On Tue, Mar 28, 2017 at 12:08 PM, Evgeny Grin <k...@yandex.ru> wrote:
> Fixed version of you code:
>
> 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) {
> srv->listening = false; close(fd); 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");
> break; } shutdown_attempts++; sleep(1); } } } else _BF_LOG("Server
> quiesce failed.\n"); MHD_stop_daemon(srv->mhd); return true; } return
> false; }
>
> --
> Best Wishes,
> Evgeny Grin
>
> On 28.03.2017 4:46, silvioprog wrote:
> > 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
> > <mailto: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
> > <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
>
--
Silvio Clécio
