Nginx fails to accept new connection if active worker crashes

[faskiri.devel]

Thanks for your attention!

It works fine with accept_mutex off, I will run my stress test harness
over the weekend to see the impact on the performance. If there is a
significant difference in performance, will surely update the thread
with the same.

For my understanding, I had implemented a workaround to get around this
problem. Is your solution along the same line?

My patch:

diff --git a/service/nginxServer/nginx-1.0.5/src/event/ngx_event.c
b/service/nginxServer/nginx-1.0.5/src/event/ngx_event.c
index c57d37e..a6ed725 100644
--- a/service/nginxServer/nginx-1.0.5/src/event/ngx_event.c
+++ b/service/nginxServer/nginx-1.0.5/src/event/ngx_event.c
@@ -49,6 +49,10 @@ ngx_atomic_t         *ngx_connection_counter =
&connection_counter;
 
 ngx_atomic_t         *ngx_accept_mutex_ptr;
 ngx_shmtx_t           ngx_accept_mutex;
+// This is shared var protected by ngx_use_accept_mutex. Access only
when
+// ngx_accept_mutex is held. The var stores the PID of the process
currently
+// holding the mutex
+ngx_pid_t            *ngx_accept_mutex_held_by;
 ngx_uint_t            ngx_use_accept_mutex;
 ngx_uint_t            ngx_accept_events;
 ngx_uint_t            ngx_accept_mutex_held;
@@ -254,6 +258,7 @@ ngx_process_events_and_timers(ngx_cycle_t *cycle)
     }
 
     if (ngx_accept_mutex_held) {
+        *ngx_accept_mutex_held_by = 0;
         ngx_shmtx_unlock(&ngx_accept_mutex);
     }
 
@@ -526,6 +531,9 @@ ngx_event_module_init(ngx_cycle_t *cycle)
     {
         return NGX_ERROR;
     }
+    // cl = 128 bytes are available for us to use. ngx_shmtx_create
uses
+    // ngx_atomic_t bytes to assign to mutex->lock, using the memory
after that
+    ngx_accept_mutex_held_by = (ngx_pid_t*) (shared +
sizeof(ngx_atomic_t));
 
     ngx_connection_counter = (ngx_atomic_t *) (shared + 1 * cl);
 
diff --git a/service/nginxServer/nginx-1.0.5/src/event/ngx_event.h
b/service/nginxServer/nginx-1.0.5/src/event/ngx_event.h
index 778da52..f1b06d4 100644
--- a/service/nginxServer/nginx-1.0.5/src/event/ngx_event.h
+++ b/service/nginxServer/nginx-1.0.5/src/event/ngx_event.h
@@ -501,6 +501,7 @@ extern ngx_atomic_t         
*ngx_connection_counter;
 
 extern ngx_atomic_t          *ngx_accept_mutex_ptr;
 extern ngx_shmtx_t            ngx_accept_mutex;
+extern ngx_pid_t             *ngx_accept_mutex_held_by;
 extern ngx_uint_t             ngx_use_accept_mutex;
 extern ngx_uint_t             ngx_accept_events;
 extern ngx_uint_t             ngx_accept_mutex_held;
diff --git
a/service/nginxServer/nginx-1.0.5/src/event/ngx_event_accept.c
b/service/nginxServer/nginx-1.0.5/src/event/ngx_event_accept.c
index 2355d1b..feb4568 100644
--- a/service/nginxServer/nginx-1.0.5/src/event/ngx_event_accept.c
+++ b/service/nginxServer/nginx-1.0.5/src/event/ngx_event_accept.c
@@ -298,6 +298,10 @@ ngx_trylock_accept_mutex(ngx_cycle_t *cycle)
         ngx_log_debug0(NGX_LOG_DEBUG_EVENT, cycle->log, 0,
                        "accept mutex locked");
 
+        *ngx_accept_mutex_held_by = ngx_pid;
+
+        // If the mutex was already held by me and we are using
RTSIG_EVENT, no
+        // need to enable accept_events

         if (ngx_accept_mutex_held
             && ngx_accept_events == 0
             && !(ngx_event_flags & NGX_USE_RTSIG_EVENT))
@@ -306,6 +310,8 @@ ngx_trylock_accept_mutex(ngx_cycle_t *cycle)
         }
 
         if (ngx_enable_accept_events(cycle) == NGX_ERROR) {
+            // No one is holding the mutex now
+            *ngx_accept_mutex_held_by = 0;
             ngx_shmtx_unlock(&ngx_accept_mutex);
             return NGX_ERROR;
         }
@@ -317,8 +323,9 @@ ngx_trylock_accept_mutex(ngx_cycle_t *cycle)
     }
 
     ngx_log_debug1(NGX_LOG_DEBUG_EVENT, cycle->log, 0,
-                   "accept mutex lock failed: %ui",
ngx_accept_mutex_held);
+                   "accept mutex lock failed: held by: %ui",
*ngx_accept_mutex_held_by);
 
+    // If I held it earlier, but not anymore (ngx_trylock_accept_mutex
failed)
     if (ngx_accept_mutex_held) {
         if (ngx_disable_accept_events(cycle) == NGX_ERROR) {
             return NGX_ERROR;
diff --git a/service/nginxServer/nginx-1.0.5/src/os/unix/ngx_process.c
b/service/nginxServer/nginx-1.0.5/src/os/unix/ngx_process.c
index 6055587..b66d4b3 100644
--- a/service/nginxServer/nginx-1.0.5/src/os/unix/ngx_process.c
+++ b/service/nginxServer/nginx-1.0.5/src/os/unix/ngx_process.c
@@ -492,17 +492,18 @@ ngx_process_get_status(void)
         }
 
 
-        if (ngx_accept_mutex_ptr) {
-
-            /*
-             * unlock the accept mutex if the abnormally exited
process
-             * held it
-             */
-
-            ngx_atomic_cmp_set(ngx_accept_mutex_ptr, pid, 0);
+        // If the accept mutex is held by the abnormally exited
process
+        // Note: If the process holding this has died, the mutex cannot
be
+        // acquired by someone else, in which case,
ngx_accept_mutex_held is
+        // free to be accessed
+        if (ngx_accept_mutex_held_by != NULL && pid ==
*ngx_accept_mutex_held_by) {
+            ngx_log_error(NGX_LOG_INFO, ngx_cycle->log, 0,
+                "PID %P held the accept mutex. Releasing", pid);
+            // Reset the value before unlocking
+            *ngx_accept_mutex_held_by = 0;
+            ngx_shmtx_unlock(&ngx_accept_mutex);
         }
 
-
         one = 1;
         process = "unknown process";
 
-- 
1.7.1

Posted at Nginx Forum: http://forum.nginx.org/read.php?2,218359,218425#msg-218425