Re: [libmicrohttpd] MHD_quiesce_daemon() question

[silvioprog]

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

> <mailto: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
>     <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