Hi there, On Mon, Apr 27, 2020 at 04:26:31PM -0700, Thibault Charbonnier wrote: > On 4/25/20 6:12 PM, Maxim Dounin wrote: > > A better approach might be to check parent's pid instead, much > > like we do when handling the changebin signal on unix (see > > src/os/unix/ngx_process.c). > > Great! Thanks for the suggestion. Below is a revised approach for the > patch (also attached to this email) which passes all of the test cases > listed in my previous test file at the start of this thread: > > # HG changeset patch > # User Thibault Charbonnier <[email protected]> > # Date 1582764433 28800 > # Wed Feb 26 16:47:13 2020 -0800 > # Node ID 8d781bac6c4feebb2d1ea3f4e6df76d71f74e43b > # 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 8d781bac6c4f 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 > @@ -1070,7 +1070,8 @@ > > if (ls[i].sockaddr->sa_family == AF_UNIX > && ngx_process <= NGX_PROCESS_MASTER > - && ngx_new_binary == 0) > + && ngx_new_binary == 0 > + && ngx_getppid() != ngx_parent) > { > u_char *name = ls[i].addr_text.data + sizeof("unix:") - 1; > > diff -r 4f18393a1d51 -r 8d781bac6c4f 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; > > 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; > } >
Thanks for your patch. Unfortunately, it would break removing of UNIX-domain socket files when nginx is run with "daemon off". It'd also add a regression that the master process will not remove the UNIX-domain socket files until after all worker processes have exited (this has been fixed in 0.1.40). The committed fixes: http://hg.nginx.org/nginx/rev/9c038f5e0464 http://hg.nginx.org/nginx/rev/7cbf6389194b _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
