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