Hello Maxim, Any thoughts on the latest version of this patch?
Cheers, On 2/27/20 4:23 PM, Thibault Charbonnier wrote: > On 2/27/20 7:24 AM, Maxim Dounin wrote: >> Have you checked what happens during binary upgrade with your >> patch? > > Good call, I gave it a thought but did not bother checking, thanks for > sharing the previous patch. This one is slightly simpler. > > Below is a new version of the patch covering binary upgrade edge-cases > by relying on the existence of the nginx.oldpid file. > > Also attached to this email is a file I used as a test suite covering > the behavior of this patch with SIGQUIT, SIGTERM, and binary upgrade > scenarios. > > # HG changeset patch > # User Thibault Charbonnier <[email protected]> > # Date 1582764433 28800 > # Wed Feb 26 16:47:13 2020 -0800 > # Node ID ec619d02801b925b4dad51515fb9668c1d993418 > # Parent 4f18393a1d51bce6103ea2f1b2587900f349ba3d > Ensured SIGQUIT deletes listening UNIX socket files. > > Prior to this patch, the SIGQUIT signal handling (graceful shutdown) did not > remove UNIX socket files since ngx_master_process_cycle reimplemented > listening > socket closings in lieu of using ngx_close_listening_sockets. > > Since ngx_master_process_exit will call the aforementioned > ngx_close_listening_sockets, we can remove the custom implementation and now > expect listening sockets to be closed properly by > ngx_close_listening_sockets > instead. > > This fixes the trac issue #753 (https://trac.nginx.org/nginx/ticket/753). > > diff -r 4f18393a1d51 -r ec619d02801b src/core/ngx_connection.c > --- a/src/core/ngx_connection.c Thu Feb 20 16:51:07 2020 +0300 > +++ b/src/core/ngx_connection.c Wed Feb 26 16:47:13 2020 -0800 > @@ -1023,6 +1023,10 @@ > ngx_uint_t i; > ngx_listening_t *ls; > ngx_connection_t *c; > +#if (NGX_HAVE_UNIX_DOMAIN) > + ngx_core_conf_t *ccf; > + ngx_fd_t fd; > +#endif > > if (ngx_event_flags & NGX_USE_IOCP_EVENT) { > return; > @@ -1067,10 +1071,22 @@ > } > > #if (NGX_HAVE_UNIX_DOMAIN) > + ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx, > ngx_core_module); > + > + fd = ngx_open_file(ccf->oldpid.data, NGX_FILE_RDONLY, > NGX_FILE_OPEN, 0); > + > + if (fd != NGX_INVALID_FILE) { > + if (ngx_close_file(fd) == NGX_FILE_ERROR) { > + ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno, > + ngx_close_file_n " \"%s\" failed", > + ccf->oldpid.data); > + } > + } > > if (ls[i].sockaddr->sa_family == AF_UNIX > && ngx_process <= NGX_PROCESS_MASTER > - && ngx_new_binary == 0) > + && ngx_new_binary == 0 > + && fd == NGX_INVALID_FILE) > { > u_char *name = ls[i].addr_text.data + sizeof("unix:") - 1; > > diff -r 4f18393a1d51 -r ec619d02801b src/os/unix/ngx_process_cycle.c > --- a/src/os/unix/ngx_process_cycle.c Thu Feb 20 16:51:07 2020 +0300 > +++ b/src/os/unix/ngx_process_cycle.c Wed Feb 26 16:47:13 2020 -0800 > @@ -77,12 +77,11 @@ > u_char *p; > size_t size; > ngx_int_t i; > - ngx_uint_t n, sigio; > + ngx_uint_t sigio; > sigset_t set; > struct itimerval itv; > ngx_uint_t live; > ngx_msec_t delay; > - ngx_listening_t *ls; > ngx_core_conf_t *ccf; > > >> Previous attempt to fix this was here: >> >> http://mailman.nginx.org/pipermail/nginx-devel/2016-December/009207.html >> http://mailman.nginx.org/pipermail/nginx-devel/2016-December/009208.html >> >> Yet it failed to address binary upgrade case properly, see here: >> >> http://mailman.nginx.org/pipermail/nginx-devel/2016-December/009239.html >> > > sigemptyset(&set); > @@ -205,16 +204,6 @@ > ngx_signal_worker_processes(cycle, > > ngx_signal_value(NGX_SHUTDOWN_SIGNAL)); > > - ls = cycle->listening.elts; > - for (n = 0; n < cycle->listening.nelts; n++) { > - if (ngx_close_socket(ls[n].fd) == -1) { > - ngx_log_error(NGX_LOG_EMERG, cycle->log, > ngx_socket_errno, > - ngx_close_socket_n " %V failed", > - &ls[n].addr_text); > - } > - } > - cycle->listening.nelts = 0; > - > continue; > } > > > _______________________________________________ > nginx-devel mailing list > [email protected] > http://mailman.nginx.org/mailman/listinfo/nginx-devel > _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
