Hi, 

enclosed you will find an attached changeset, that contains fix for
windows issue with multiple workers (once listening - only one made any
work). 

If someone needs a git version of it: 

https://github.com/sebres/nginx/pull/1/files [1] 

Here [2] you may find a benchmark comparison for that (1 worker column -
measured before fix). 

-- 

Shortly about fix algorithm (changes related are marked as [*],
unchanged - [-]): 

- master process create all listener; 

- [cycle] master process create a worker; 

* [win32] master calls `ngx_share_listening_sockets`: each listener
share (inheritance) info for pid of this worker ("cloned" via
WSADuplicateSocket), that will be saved in shmem; 

- master process wait until worker will send an event "worker_nnn"; 

* [win32] worker process executes `ngx_get_listening_share_info` to
obtain shared info, protocol structure that can be used to create a new
socket descriptor for a shared socket; 

* [win32] worker process creates all listener sockets using given shared
info of master; 

- worker process sets an event "worker_nnn"; 

- master process create next worker, repeat [cycle]. 

-- 

@Maxim Dounin:
1) your suggested way with shared handle and bInheritHandle does not
work, because of:
[quote]
Sockets. No error is returned, but the duplicate handle may not be
recognized by Winsock at the target process. Also, using DUPLICATEHANDLE
interferes with internal reference counting on the underlying object. 
To duplicate a socket handle, use the WSADUPLICATESOCKET function.
[/quote] 

2) proposal to use an environment instead of shared memory can not work
also, because sharing via WSADuplicateSocket should already know a pid
of target process, that uses this handle - specially shared for each
worker. 

BTW, using of `accept_mutex` was disallowed for win32, cause of possible
deadlock if grabbed by a process which can't accept connections.
Because, this is fixed now, I have removed this "restriction" in
separated commit.
But I think, accept_mutex is not needed in win32 resp. with accept_mutex
it is much slower as without him. So, whats about set default of
`accept_mutex` to `off` on windows platform? 

BTW[2], I have executed extensive tests of this fix, also with reloading
(increase/decrease `worker_processes`), restarting, as well as
auto-restarting of worker, if it was "crashed" (ex.: have sporadically
killed some worker). 

Regards,
Serg G. Brester (aka sebres). 

 

Links:
------
[1] https://github.com/sebres/nginx/pull/1/files
[2] https://github.com/sebres/nginx/pull/1
# HG changeset patch
# User sebres <serg.bres...@sebres.de>
# Date 1433956047 -7200
#      Wed Jun 10 19:07:27 2015 +0200
# Node ID 7f0a48e380944d476063419db7e99122e2f17d92
# Parent  1729d8d3eb3acbb79b1b0c1d60b411aacc4f8461
prevent extreme growth of log file if select failed (possible "nowait" inside)

diff -r 1729d8d3eb3a -r 7f0a48e38094 src/event/modules/ngx_win32_select_module.c
--- a/src/event/modules/ngx_win32_select_module.c	Mon Jun 08 23:13:56 2015 +0300
+++ b/src/event/modules/ngx_win32_select_module.c	Wed Jun 10 19:07:27 2015 +0200
@@ -214,6 +214,8 @@ ngx_select_del_event(ngx_event_t *ev, ng
 }
 
 
+static ngx_uint_t ngx_select_err_cntr = 0;
+
 static ngx_int_t
 ngx_select_process_events(ngx_cycle_t *cycle, ngx_msec_t timer,
     ngx_uint_t flags)
@@ -278,7 +280,16 @@ ngx_select_process_events(ngx_cycle_t *c
                    "select ready %d", ready);
 
     if (err) {
-        ngx_log_error(NGX_LOG_ALERT, cycle->log, err, "select() failed");
+        /* because select failed (possible nowait) - prevent extreme growth of log file */
+        if (++ngx_select_err_cntr < 10) {
+            ngx_log_error(NGX_LOG_ALERT, cycle->log, err, "select() failed");
+        } else if (ngx_select_err_cntr == 10) {
+            ngx_log_error(NGX_LOG_ALERT, cycle->log, err, "select() failed," 
+								" %d times - no more log)", 10);
+        } else {
+            /* prevent 100% cpu usage if nowait - WSAEINVAL etc. */
+            ngx_msleep(500);
+        }
 
         if (err == WSAENOTSOCK) {
             ngx_select_repair_fd_sets(cycle);
@@ -286,6 +297,7 @@ ngx_select_process_events(ngx_cycle_t *c
 
         return NGX_ERROR;
     }
+    ngx_select_err_cntr = 0;
 
     if (ready == 0) {
         if (timer != NGX_TIMER_INFINITE) {
# HG changeset patch
# User sebres <serg.bres...@sebres.de>
# Date 1433956100 -7200
#      Wed Jun 10 19:08:20 2015 +0200
# Node ID 80e20498d99e0a4065dfd5ec5c1633cdcfc94541
# Parent  7f0a48e380944d476063419db7e99122e2f17d92
Fix windows issue with multiple workers (once listening - only one made any work),
using shared socket handles for each worker process (WSADuplicateHandle).

diff -r 7f0a48e38094 -r 80e20498d99e src/core/ngx_connection.c
--- a/src/core/ngx_connection.c	Wed Jun 10 19:07:27 2015 +0200
+++ b/src/core/ngx_connection.c	Wed Jun 10 19:08:20 2015 +0200
@@ -369,6 +369,9 @@ ngx_open_listening_sockets(ngx_cycle_t *
     ngx_log_t        *log;
     ngx_socket_t      s;
     ngx_listening_t  *ls;
+#if (NGX_WIN32)
+    ngx_shared_socket_info  shinfo = NULL;
+#endif
 
     reuseaddr = 1;
 #if (NGX_SUPPRESS_WARN)
@@ -420,6 +423,40 @@ ngx_open_listening_sockets(ngx_cycle_t *
                 continue;
             }
 
+#if (NGX_WIN32)
+            /* try to use shared sockets of master */
+            if (ngx_process > NGX_PROCESS_MASTER) {
+                
+                if (!shinfo) {
+                    shinfo = ngx_get_listening_share_info(cycle, ngx_getpid());
+
+                    if (!shinfo) {
+                        failed = 1;
+                        break;
+                    }
+                }
+
+                s = ngx_shared_socket(ls[i].sockaddr->sa_family, ls[i].type, 0,
+                    shinfo+i);
+
+                if (s == (ngx_socket_t) -1) {
+                    ngx_log_error(NGX_LOG_EMERG, log, ngx_socket_errno,
+                                  ngx_socket_n " for inherited socket %V failed", 
+                                  &ls[i].addr_text);
+                    return NGX_ERROR;
+                }
+                    
+                ngx_log_debug4(NGX_LOG_DEBUG_CORE, log, 0, "[%d] shared socket %d %V: %d",
+                    ngx_process, i, &ls[i].addr_text, s);
+
+                ls[i].fd = s;
+
+                ls[i].listen = 1;
+
+                continue;
+            }
+#endif
+
             if (ls[i].inherited) {
 
                 /* TODO: close on exit */
@@ -430,6 +467,8 @@ ngx_open_listening_sockets(ngx_cycle_t *
             }
 
             s = ngx_socket(ls[i].sockaddr->sa_family, ls[i].type, 0);
+            ngx_log_debug4(NGX_LOG_DEBUG_CORE, log, 0, "[%d] listener %d %V: %d",
+                ngx_process, i, &ls[i].addr_text, s);
 
             if (s == (ngx_socket_t) -1) {
                 ngx_log_error(NGX_LOG_EMERG, log, ngx_socket_errno,
@@ -864,6 +903,9 @@ ngx_close_listening_sockets(ngx_cycle_t 
         return;
     }
 
+    ngx_log_debug2(NGX_LOG_DEBUG_CORE, cycle->log, 0, "[%d] close %d listener(s)",
+        ngx_process, cycle->listening.nelts);
+
     ngx_accept_mutex_held = 0;
     ngx_use_accept_mutex = 0;
 
diff -r 7f0a48e38094 -r 80e20498d99e src/os/win32/ngx_process.c
--- a/src/os/win32/ngx_process.c	Wed Jun 10 19:07:27 2015 +0200
+++ b/src/os/win32/ngx_process.c	Wed Jun 10 19:08:20 2015 +0200
@@ -70,6 +70,10 @@ ngx_spawn_process(ngx_cycle_t *cycle, ch
         return pid;
     }
 
+#if (NGX_WIN32)
+    ngx_share_listening_sockets(cycle, pid);
+#endif
+
     ngx_memzero(&ngx_processes[s], sizeof(ngx_process_t));
 
     ngx_processes[s].handle = ctx.child;
diff -r 7f0a48e38094 -r 80e20498d99e src/os/win32/ngx_process_cycle.c
--- a/src/os/win32/ngx_process_cycle.c	Wed Jun 10 19:07:27 2015 +0200
+++ b/src/os/win32/ngx_process_cycle.c	Wed Jun 10 19:08:20 2015 +0200
@@ -113,7 +113,10 @@ ngx_master_process_cycle(ngx_cycle_t *cy
     events[2] = ngx_reopen_event;
     events[3] = ngx_reload_event;
 
+/* does not close listener for win32, will be shared */
+#if (!NGX_WIN32)
     ngx_close_listening_sockets(cycle);
+#endif
 
     if (ngx_start_worker_processes(cycle, NGX_PROCESS_RESPAWN) == 0) {
         exit(2);
@@ -207,7 +210,10 @@ ngx_master_process_cycle(ngx_cycle_t *cy
 
             ngx_cycle = cycle;
 
+/* does not close listener for win32, will be shared */
+#if (!NGX_WIN32)
             ngx_close_listening_sockets(cycle);
+#endif
 
             if (ngx_start_worker_processes(cycle, NGX_PROCESS_JUST_RESPAWN)) {
                 ngx_quit_worker_processes(cycle, 1);
diff -r 7f0a48e38094 -r 80e20498d99e src/os/win32/ngx_socket.c
--- a/src/os/win32/ngx_socket.c	Wed Jun 10 19:07:27 2015 +0200
+++ b/src/os/win32/ngx_socket.c	Wed Jun 10 19:08:20 2015 +0200
@@ -9,6 +9,18 @@
 #include <ngx_core.h>
 
 
+typedef struct {
+
+    ngx_pid_t      pid;
+    ngx_uint_t     nelts;
+
+    /* WSAPROTOCOL_INFO * [listening.nelts] */
+
+} ngx_shm_listener_t;
+
+static ngx_shm_t shm_listener = {0};
+
+
 int
 ngx_nonblocking(ngx_socket_t s)
 {
@@ -32,3 +44,129 @@ ngx_tcp_push(ngx_socket_t s)
 {
     return 0;
 }
+
+
+ngx_int_t ngx_get_listening_share(ngx_cycle_t *cycle)
+{
+
+    size_t            size;
+
+    size = sizeof(ngx_shm_listener_t) +
+        sizeof(WSAPROTOCOL_INFO) * cycle->listening.nelts;
+
+    if (!shm_listener.addr || shm_listener.size < size) {
+        if (shm_listener.addr) {
+            ngx_shm_free(&shm_listener);
+        }
+
+        shm_listener.addr = NULL;
+        shm_listener.size = size;
+        shm_listener.name.len = sizeof("nginx_shared_listener") - 1;
+        shm_listener.name.data = (u_char *) "nginx_shared_listener";
+        shm_listener.log = cycle->log;
+
+        if (ngx_shm_alloc(&shm_listener) != NGX_OK) {
+            return NGX_ERROR;
+        }
+        
+        ngx_log_debug4(NGX_LOG_DEBUG_CORE, cycle->log, 0, 
+            "[%d] shared mem for %d listener(s) - %p, %d bytes", 
+            ngx_process, cycle->listening.nelts, 
+            shm_listener.addr, shm_listener.size);
+    }
+
+    return NGX_OK;
+}
+
+
+ngx_shared_socket_info 
+ngx_get_listening_share_info(ngx_cycle_t *cycle, ngx_pid_t pid)
+{
+    ngx_int_t            waitcnt;
+    ngx_shm_listener_t  *shml;
+
+    if (shm_listener.addr == NULL) {
+        if (ngx_get_listening_share(cycle) != NGX_OK) {
+            return NULL;
+        }
+    }
+
+    /* TODO: wait time and count configurable */
+    waitcnt = 5;
+    do {
+    
+        shml = (ngx_shm_listener_t *)shm_listener.addr;
+        if (shml->pid == pid) {
+            break;
+        }
+        /* not found - wait until master process shared sockets */
+        ngx_msleep(100);
+    
+    } while (waitcnt--);
+
+    if (shml->pid != pid) {
+        ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, 
+            "wait for shared socket failed, process %d, found %d", pid, shml->pid);
+        return NULL;
+    }
+    if (cycle->listening.nelts > shml->nelts) {
+        ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, "unexpected shared len,"
+            " expected %d, but got %d", cycle->listening.nelts, shml->nelts);
+        return NULL;
+    }
+    
+    return (WSAPROTOCOL_INFO *)(shml+1);
+}
+
+
+ngx_int_t 
+ngx_share_listening_sockets(ngx_cycle_t *cycle, ngx_pid_t pid)
+{
+    ngx_uint_t           i;
+    ngx_listening_t     *ls;
+    ngx_shm_listener_t  *shml;
+    WSAPROTOCOL_INFO    *protoInfo;
+
+    if (ngx_process > NGX_PROCESS_MASTER) {
+        return NGX_OK;
+    }
+
+    ls = cycle->listening.elts;
+
+    /* create shared memory for shared listener info */
+    ngx_get_listening_share(cycle);
+
+    ngx_log_debug3(NGX_LOG_DEBUG_CORE, cycle->log, 0, 
+        "[%d] share %d listener(s) for %d",
+        ngx_process, cycle->listening.nelts, pid);
+
+    if (!cycle->listening.nelts)
+        return NGX_OK;
+
+    /* share sockets for worker with pid */
+    shml = (ngx_shm_listener_t *)shm_listener.addr;
+    protoInfo = (WSAPROTOCOL_INFO *)(shml+1);
+
+    shml->nelts = cycle->listening.nelts;
+
+    for (i = 0; i < cycle->listening.nelts; i++) {
+
+        if (ls[i].ignore) {
+            continue;
+        }
+
+        ngx_log_debug4(NGX_LOG_DEBUG_CORE, cycle->log, 0, 
+            "[%d] dup %d listener %d for %d", ngx_process, i, ls[i].fd, pid);
+
+        if (WSADuplicateSocket(ls[i].fd, pid, &protoInfo[i]) != 0) {
+            ngx_log_error(NGX_LOG_EMERG, cycle->log, ngx_socket_errno,
+                "WSADuplicateSocket() failed");
+            return NGX_ERROR;
+        };
+
+    }
+
+    shml->pid = pid;
+
+    return NGX_OK;
+}
diff -r 7f0a48e38094 -r 80e20498d99e src/os/win32/ngx_socket.h
--- a/src/os/win32/ngx_socket.h	Wed Jun 10 19:07:27 2015 +0200
+++ b/src/os/win32/ngx_socket.h	Wed Jun 10 19:08:20 2015 +0200
@@ -11,6 +11,7 @@
 
 #include <ngx_config.h>
 #include <ngx_core.h>
+#include <ngx_process.h>
 
 
 #define NGX_WRITE_SHUTDOWN SD_SEND
@@ -204,4 +205,13 @@ int ngx_tcp_push(ngx_socket_t s);
 #define ngx_tcp_push_n            "tcp_push()"
 
 
+typedef WSAPROTOCOL_INFO * ngx_shared_socket_info;
+#define ngx_shared_socket(af, type, proto, shinfo)                           \
+    WSASocket(af, type, proto, shinfo, 0, WSA_FLAG_OVERLAPPED)
+
+ngx_shared_socket_info ngx_get_listening_share_info(ngx_cycle_t *cycle, 
+    ngx_pid_t pid);
+ngx_int_t ngx_share_listening_sockets(ngx_cycle_t *cycle, ngx_pid_t pid);
+
+
 #endif /* _NGX_SOCKET_H_INCLUDED_ */
# HG changeset patch
# User sebres <serg.bres...@sebres.de>
# Date 1433956334 -7200
#      Wed Jun 10 19:12:14 2015 +0200
# Node ID 7f956d9152a99a5df7d2b388a8eef64729722efd
# Parent  80e20498d99e0a4065dfd5ec5c1633cdcfc94541
allow accept mutex on win32 (no more deadlock - all workers can accept connections)

diff -r 80e20498d99e -r 7f956d9152a9 src/event/ngx_event.c
--- a/src/event/ngx_event.c	Wed Jun 10 19:08:20 2015 +0200
+++ b/src/event/ngx_event.c	Wed Jun 10 19:12:14 2015 +0200
@@ -588,17 +588,6 @@ ngx_event_process_init(ngx_cycle_t *cycl
         ngx_use_accept_mutex = 0;
     }
 
-#if (NGX_WIN32)
-
-    /*
-     * disable accept mutex on win32 as it may cause deadlock if
-     * grabbed by a process which can't accept connections
-     */
-
-    ngx_use_accept_mutex = 0;
-
-#endif
-
     ngx_queue_init(&ngx_posted_accept_events);
     ngx_queue_init(&ngx_posted_events);
 
# HG changeset patch
# User sebres <serg.bres...@sebres.de>
# Date 1433957958 -7200
#      Wed Jun 10 19:39:18 2015 +0200
# Node ID e40ee60150e47616d86fdee90f62f0f88c4b1e80
# Parent  7f956d9152a99a5df7d2b388a8eef64729722efd
start worker faster - minimize overhead through wait latency;

diff -r 7f956d9152a9 -r e40ee60150e4 src/os/win32/ngx_socket.c
--- a/src/os/win32/ngx_socket.c	Wed Jun 10 19:12:14 2015 +0200
+++ b/src/os/win32/ngx_socket.c	Wed Jun 10 19:39:18 2015 +0200
@@ -82,6 +82,7 @@ ngx_int_t ngx_get_listening_share(ngx_cy
 ngx_shared_socket_info 
 ngx_get_listening_share_info(ngx_cycle_t *cycle, ngx_pid_t pid)
 {
+    ngx_int_t            waitint;
     ngx_int_t            waitcnt;
     ngx_shm_listener_t  *shml;
 
@@ -92,7 +93,8 @@ ngx_get_listening_share_info(ngx_cycle_t
     }
 
     /* TODO: wait time and count configurable */
-    waitcnt = 5;
+    waitcnt = 10;
+    waitint = 5;
     do {
     
         shml = (ngx_shm_listener_t *)shm_listener.addr;
@@ -100,7 +102,11 @@ ngx_get_listening_share_info(ngx_cycle_t
             break;
         }
         /* not found - wait until master process shared sockets */
-        ngx_msleep(100);
+        ngx_msleep(waitint);
+        if (waitint < 100) {
+            waitint += waitint; 
+            waitint = min(100, waitint);
+        }
     
     } while (waitcnt--);
 
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Reply via email to