worker process caused 100% CPU usage on one core on win32

[Nos Apatka]

Timing issue scenario:

source code file (tip level): \src\os\win32\ngx_process_cycle.c

Step 1: After the event “ngx_worker_quit_<pid>” has been signaled the code in function ngx_worker_process_cycle() of the worker process main thread set the ngx_quit to the value 1 in line #703 and jump out of the cycle;

Step 2: In ngx_worker_thread() the ngx_quit is set to the value 0 in line #840, then closing listening sockets and set the ngx_exiting = 1 in line #844.

Step 3: ngx_cache_manager_event is signaled in line #730 in function ngx_worker_process_cycle() of the worker process main thread;

Step 4: the cache manager thread has been waked up in ngx_cache_manager_process_handler() and returns back to ngx_cache_manager_thread(), but since both, ngx_terminate and ngx_quit are 0 it enters again ngx_cache_manager_process_handler() in line #944;

Step 5: since ngx_cache_manager_event remains in signaled state the WaitForSingleObject in line #982 will return immediately to ngx_cache_manager_thread() and the loop continues since ngx_terminate and ngx_quit are still 0; this thread will cause one core 100% usage;

Meanwhile, the ngx_worker_thread() will terminate and the main thread will be stuck waiting for cache manager thread to terminate in the for(;;) cycle line #740.

The solution to the issue either to check for ngx_exiting in line #939 or to set ngx_quit = 1 just before jumping out of the loop in line #827 of the worker thread. The second approach has been successfully tested on the customer system where the issue is often reproducible.

[Nos Apatka]

modified source code

[Nos Apatka]

screen shot showing issue

[Maxim Dounin]

Thanks for the report. It looks like a fallout of the fact that cache manager is a thread on win32 (instead of a normal process as it used to be on unix systems).

I don't really like the idea of rearming ngx_quit, as it's looks semantically incorrect. Checking ngx_exiting probably will be good enough though, please see the patch below:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1395862308 -14400
#      Wed Mar 26 23:31:48 2014 +0400
# Node ID 734f0babfc133c2dc532f2794deadcf9d90245f7
# Parent  384c5cb7042d9770d688852dda2beee5f0f9587c
Win32: fixed cpu hog by cache manager on exit (ticket #514).

The "ngx_quit" may be reset by the worker thread before it's seen
by a ngx_cache_manager_thread(), resulting in an infinite loop.  Make
sure to test ngx_exiting as well.

diff --git a/src/os/win32/ngx_process_cycle.c b/src/os/win32/ngx_process_cycle.c
--- a/src/os/win32/ngx_process_cycle.c
+++ b/src/os/win32/ngx_process_cycle.c
@@ -926,7 +926,7 @@ ngx_cache_manager_thread(void *data)
          * ev == WAIT_ABANDONED_0 + 1
          */
 
-        if (ngx_terminate || ngx_quit) {
+        if (ngx_terminate || ngx_quit || ngx_exiting) {
             ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "exiting");
             return 0;
         }
@@ -936,7 +936,7 @@ ngx_cache_manager_thread(void *data)
 
     for ( ;; ) {
 
-        if (ngx_terminate || ngx_quit) {
+        if (ngx_terminate || ngx_quit || ngx_exiting) {
             ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "exiting");
             break;
         }
@@ -1005,7 +1005,7 @@ ngx_cache_loader_thread(void *data)
     path = cycle->paths.elts;
     for (i = 0; i < cycle->paths.nelts; i++) {
 
-        if (ngx_terminate || ngx_quit) {
+        if (ngx_terminate || ngx_quit || ngx_exiting) {
             break;
         }
 

(Please also take a look at ​http://nginx.org/en/docs/contributing_changes.html for further reference. Trac isn't the best place to submit patches.)

[Maxim Dounin <mdounin@…>]

In b74f1106f920fe9e447c710e57a5ccdeae46d8e3/nginx:

Win32: fixed cpu hog by cache manager on exit (ticket #514).

The "ngx_quit" may be reset by the worker thread before it's seen
by a ngx_cache_manager_thread(), resulting in an infinite loop. Make
sure to test ngx_exiting as well.

[Maxim Dounin]

Fix committed, thanks.