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 <thibaultcha@me.com>
> # 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
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel