Welcome! Log In Create A New Profile

Advanced

Re: [Patch] possible mutex starvation issue affects all nginx Linux versions.

Maxim Dounin
December 02, 2013 08:40AM
Hello!

On Mon, Dec 02, 2013 at 07:15:52AM -0500, itpp2012 wrote:

> Here is a patch for a possible mutex starvation issue which affects all
> nginx Linux versions.
> Already solved for Windows since nginx 1.5.7.1 Caterpillar.
> Can be reproduced when nginx reloads the config & worker holding mutex dies
> or hangs.
>
> Fixed by Vittorio Francesco Digilio, commercially sponsered solution by
> ITPP.
> target source mainline 1.5.8 - 30-11-2013.
> src/event/ngx_event_accept.c, line 402 was correctly found (starvation fix)
> but missed in:
> src/event/ngx_event.c, line 259-260, 1 line added;
> 255 if (ngx_posted_accept_events) {
> 256 ngx_event_process_posted(cycle, &ngx_posted_accept_events);
> 257 }
> 258
> 259 if (ngx_accept_mutex_held) {
> --+ ngx_accept_mutex_held=0;
> 260 ngx_shmtx_unlock(&ngx_accept_mutex);
> 261 }
> 262
> 263 if (delta) {
> 264 ngx_event_expire_timers();
> 265 }

This patch is wrong. The ngx_accept_mutex_held don't need to be
unset here. The ngx_accept_mutex_held is used as an idicator that on
previous iteration the lock was held by the process, and unsetting
it will result in incorrect behaviour.

> Also applies to; src/os/win32/ngx_process_cycle.c
> line 507-508, 3 lines added;
> 504 if (ngx_processes[n].handle != h) {
> 505 continue;
> 506 }
> 507
> --+ if(*ngx_accept_mutex.lock==ngx_processes[n].pid) {
> --+ *ngx_accept_mutex.lock=0;
> --+ }
> 508 if (GetExitCodeProcess(h, &code) == 0) {
> 509 ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
> 510 "GetExitCodeProcess(%P) failed",
> 511 ngx_processes[n].pid);
> 512 }

This patch is also wrong. In the current state of the win32
version it is not assumed that accept mutex is used at all. But
if it is, correct aproach to unlock shared memory mutexes on
abnormal process termination is to port (or move to
platform-independed place) the ngx_unlock_mutexes() function from
src/os/unix/ngx_process.c. It correctly uses
ngx_shmtx_force_unlock() to properly unlock shared memory mutexes
using atomic operations.

Note well that unlocking shared memory mutexes on abnormal
process termination is an emergency mechanism. If it actually
happens, it indicate that something is really wrong in other
places of the system.

Please also consider reading
http://nginx.org/en/docs/contributing_changes.html.

--
Maxim Dounin
http://nginx.org/en/donation.html

_______________________________________________
nginx mailing list
nginx@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx
Subject Author Posted

[Patch] possible mutex starvation issue affects all nginx Linux versions.

itpp2012 December 02, 2013 07:15AM

Re: [Patch] possible mutex starvation issue affects all nginx Linux versions.

Maxim Dounin December 02, 2013 08:40AM

Re: [Patch] possible mutex starvation issue affects all nginx Linux versions.

itpp2012 December 11, 2013 07:57AM

Re: [Patch] possible mutex starvation issue affects all nginx Linux versions.

Maxim Dounin December 11, 2013 12:12PM



Sorry, only registered users may post in this forum.

Click here to login

Online Users

Guests: 250
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready