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; }
test.sh
Description: application/shellscript
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
