Re: патч для nginx/win32
Илья Шипицин
chipitsine at gmail.com
Thu Mar 28 12:03:05 UTC 2013
Добрый день!
поправил 0 --> TRUE в WaitForMultipleObjects, чтобы дождаться
завершения всех потомков, патч:
--- src/os/win32/ngx_process_cycle.c.orig 2013-03-27
09:53:48.000000000 +0600
+++ src/os/win32/ngx_process_cycle.c 2013-03-28 17:58:49.000000000 +0600
@@ -303,7 +303,10 @@
ngx_console_handler(u_long type)
{
char *msg;
-
+ ngx_int_t n;
+ u_long nev, timeout, ev;
+ HANDLE events[MAXIMUM_WAIT_OBJECTS];
+
switch (type) {
case CTRL_C_EVENT:
@@ -337,6 +340,18 @@
"SetEvent(\"%s\") failed", ngx_stop_event_name);
}
+ nev = 0;
+ for (n = 0; n < ngx_last_process; n++) {
+ if (ngx_processes[n].handle) {
+ events[nev++] = ngx_processes[n].handle;
+ }
+ }
+
+ if(nev != 0){
+ timeout = INFINITE;
+ ev = WaitForMultipleObjects(nev, events, TRUE, timeout);
+ }
+
return 1;
}
28 марта 2013 г., 17:22 пользователь Илья Шипицин
<chipitsine at gmail.com> написал:
> Добрый день!
>
> ngx_sleep я вставлял перед SetEvent, вставил после - да, сработало.
> проверьте очередной патч (задумка такая - ждем завершения потомков,
> если они есть, после чего выходим из ngx_console_close)
>
> насчет HTML - гмейл так по-умолчанию делает. всё против вас настроено ))
>
>
> --- src/os/win32/ngx_process_cycle.c.orig 2013-03-27
> 09:53:48.000000000 +0600
> +++ src/os/win32/ngx_process_cycle.c 2013-03-28 17:14:17.000000000 +0600
> @@ -303,7 +303,10 @@
> ngx_console_handler(u_long type)
> {
> char *msg;
> -
> + ngx_int_t n;
> + u_long nev, timeout, ev;
> + HANDLE events[MAXIMUM_WAIT_OBJECTS];
> +
> switch (type) {
>
> case CTRL_C_EVENT:
> @@ -337,6 +340,18 @@
> "SetEvent(\"%s\") failed", ngx_stop_event_name);
> }
>
> + nev = 0;
> + for (n = 0; n < ngx_last_process; n++) {
> + if (ngx_processes[n].handle) {
> + events[nev++] = ngx_processes[n].handle;
> + }
> + }
> +
> + if(nev != 0){
> + timeout = INFINITE;
> + ev = WaitForMultipleObjects(nev, events, 0, timeout);
> + }
> +
> return 1;
> }
>
> 27 марта 2013 г., 19:38 пользователь Maxim Dounin <mdounin at mdounin.ru> написал:
>> Hello!
>>
>> On Wed, Mar 27, 2013 at 06:57:05PM +0600, Илья Шипицин wrote:
>>
>>> я проверил банальный ngx_msleep(1000) - он не помогает.
>>>
>>> вот тут
>>>
>>> http://msdn.microsoft.com/ru-RU/library/windows/desktop/ms686722%28v=vs.85%29.aspx
>>>
>>> написано " When the system is terminating a process, it does not terminate
>>> any child processes that the process has created."
>>
>> Судя по цитируемому - понимания, почему ngx_msleep() должет
>> помочь, не наступило, и подозреваю, что проверка была
>> неправильной.
>>
>> На пальцах:
>>
>> Ниже в этой же функции nginx зовёт SetEvent(ngx_stop_event).
>> Это, в свою очередь, должно вызвать возврат из
>> WaitForMultipleObjects() в ngx_master_process_cycle(), и завершение
>> рабочих процессов.
>>
>> Наша задача в ngx_console_handler() - дождаться, пока это
>> случится, и только после этого вернуть 1.
>>
>> Подробнее о том, как система вызывает обработчики, установленные с
>> помощью SetConsoleCtrlHandler(), можно почитать тут:
>>
>> http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx
>>
>> (И таки да, html, top-posting и несколько ответов на одно и то же
>> письмо - это всё хорошие способы убедить меня не отвечать.)
>>
>>> 27 марта 2013 г., 18:20 пользователь Maxim Dounin <mdounin at mdounin.ru>написал:
>>>
>>> > Hello!
>>> >
>>> > On Wed, Mar 27, 2013 at 09:58:07AM +0600, Илья Шипицин wrote:
>>> >
>>> > > вот такой вариант ?
>>> > >
>>> > > --- src/os/win32/ngx_process_cycle.c.orig 2013-03-27
>>> > > 09:53:48.000000000 +0600
>>> > > +++ src/os/win32/ngx_process_cycle.c 2013-03-27 09:48:56.000000000
>>> > +0600
>>> > > @@ -303,6 +303,8 @@
>>> > > ngx_console_handler(u_long type)
>>> > > {
>>> > > char *msg;
>>> > > + ngx_cycle_t *cycle;
>>> > > + cycle = (ngx_cycle_t *) ngx_cycle;
>>> > >
>>> > > switch (type) {
>>> > >
>>> > > @@ -316,6 +318,8 @@
>>> > >
>>> > > case CTRL_CLOSE_EVENT:
>>> > > msg = "console closing, exiting";
>>> > > + ngx_quit = 1;
>>> > > + ngx_quit_worker_processes(cycle, 0);
>>> > > break;
>>> > >
>>> > > case CTRL_LOGOFF_EVENT:
>>> >
>>> > Я пересморел этот код ещё раз, перечитал соответствующую виндовую
>>> > документацию, и склонен думать, что:
>>> >
>>> > 1) Патч, меняющий обработку только в случае CTRL_CLOSE_EVENT -
>>> > заведомое неправильный, т.к. все случаи с точки зрения системы и
>>> > nginx'а - равнозначны. (Прозвучавшее тут утверждение, что по
>>> > Ctrl-C воркеры закрываются - видимо основано на наблюдениях за
>>> > отдельными случаями, когда везло. Текущее поведение - содержит в
>>> > себе race, см. ниже, и может иногда работать правильно.)
>>> >
>>> > 2) Текущая обработка - правильная, за одним небольшим упущением:
>>> > нужно дожидаться завершения процесса до собственно возврата из
>>> > ngx_console_handler(), потому что после возврата - процесс
>>> > завершат, и сделанная нами попытка запустить процедуру штатной
>>> > остановки - скорее всего пропадёт впустую. Что, собственно, и
>>> > происходит.
>>> >
>>> > IMHO, правильным решением будет добавить ожидание перед возвратом
>>> > из ngx_console_handler(). В качетсве грубого хака - можно
>>> > попробовать воткнуть туда банальный ngx_msleep(1000), должно
>>> > помочь.
>>> >
>>> > >
>>> > >
>>> > >
>>> > >
>>> > >
>>> > >
>>> > > 26 марта 2013 г., 18:08 пользователь Maxim Dounin <mdounin at mdounin.ru
>>> > >написал:
>>> > >
>>> > > > Hello!
>>> > > >
>>> > > > On Tue, Mar 26, 2013 at 05:43:21PM +0600, Илья Шипицин wrote:
>>> > > >
>>> > > > > давайте разбираться. если запускать nginx в консоли (это штатный
>>> > режим,
>>> > > > так
>>> > > > > работают назначенные задания), то завершение задания с точки зрения
>>> > > > > мастер-процесса выглядит, как CTRL_CLOSE_EVENT в функции-обработчике
>>> > > > > ngx_console_handler
>>> > > > >
>>> > > > > worker-процесс в это время залипает в функции
>>> > ngx_worker_process_cycle в
>>> > > > > цикле "ev=WaitForMultipleObjects()"
>>> > > > >
>>> > > > > соответственно, закрытие мастера путем закрывания не приводит к тому,
>>> > > > что в
>>> > > > > данном месте возникает какое-то событие.
>>> > > >
>>> > > > То, что это плохо - вопросов не вызывает. По Ctrl-C всё должно
>>> > > > штатно закрываться, а не висеть вечно.
>>> > > >
>>> > > > > варианты - либо существенно переделывать логику и протаскивать сюда
>>> > еще
>>> > > > > одно событие, либо жестко закрыть worker через
>>> > > > > ngx_terminate_worker_processes.
>>> > > > >
>>> > > > > чем чреват второй вариант ? ну ок, закроются текущие сессии. завершая
>>> > > > > задание, мы, вероятно, этого и добиваемся.
>>> > > >
>>> > > > Например, могут остаться полусохранённые файлы в кеше/proxy_store -
>>> > > > если рабочий процесс прервали в процессе копирования временного
>>> > > > файла в целевой каталог.
>>> > > >
>>> > > > (Документация по TerminateProcess() и различные code
>>> > > > checker'ы любят пугать про "the state of global data maintained
>>> > > > by dynamic-link libraries (DLLs) may be compromised". Но это,
>>> > > > насколько я понимаю, в данном случае к nginx'у неприменимо - по
>>> > > > крайней мере, в отсутствии сторонних модулей.)
>>> > > >
>>> > > > >
>>> > > > >
>>> > > > > 26 марта 2013 г., 17:27 пользователь Maxim Dounin <
>>> > mdounin at mdounin.ru
>>> > > > >написал:
>>> > > > >
>>> > > > > > Hello!
>>> > > > > >
>>> > > > > > On Tue, Mar 26, 2013 at 05:03:30PM +0600, Илья Шипицин wrote:
>>> > > > > >
>>> > > > > > > Добрый день!
>>> > > > > > >
>>> > > > > > > мы достаточно плотно используем nginx для Windows, запускаем его
>>> > > > через
>>> > > > > > > назначенное задание (scheduled tasks). Для этого в конфиге надо
>>> > > > сделать
>>> > > > > > > "daemon off" и дальше менеджер заданий следит за
>>> > мастер-процессом,
>>> > > > > > > запущенным на терминале.
>>> > > > > > >
>>> > > > > > > это, кстати, удобнее, чем служба Windows (вообще, назначенные
>>> > задания
>>> > > > > > более
>>> > > > > > > удобны и мы чаще используем их, чем службы).
>>> > > > > > >
>>> > > > > > > в этом сценарии есть один недостаток, при завершении
>>> > мастер-процесса,
>>> > > > > > > остается запущенный worker-процесс.
>>> > > > > > >
>>> > > > > > > насколько я понял, в случае Windows это штатная ситуация (при
>>> > такой
>>> > > > > > работе
>>> > > > > > > с процессами, которая используется в nginx), для исправления
>>> > > > предлагаю
>>> > > > > > > такой патч (сделан для 1.3.14):
>>> > > > > > >
>>> > > > > > > --- src/os/win32/ngx_process_cycle.c 2013-03-26
>>> > 16:57:20.000000000
>>> > > > > > +0600
>>> > > > > > > +++ src/os/win32/ngx_process_cycle.c.new 2013-03-26
>>> > > > > > > 16:57:00.987341331 +0600
>>> > > > > > > @@ -303,6 +303,8 @@
>>> > > > > > > ngx_console_handler(u_long type)
>>> > > > > > > {
>>> > > > > > > char *msg;
>>> > > > > > > + ngx_cycle_t *cycle;
>>> > > > > > > + cycle = (ngx_cycle_t *) ngx_cycle;
>>> > > > > > >
>>> > > > > > > switch (type) {
>>> > > > > > >
>>> > > > > > > @@ -316,6 +318,7 @@
>>> > > > > > >
>>> > > > > > > case CTRL_CLOSE_EVENT:
>>> > > > > > > msg = "console closing, exiting";
>>> > > > > > > + ngx_terminate_worker_processes(cycle);
>>> > > > > > > break;
>>> > > > > > >
>>> > > > > > > case CTRL_LOGOFF_EVENT:
>>> > > > > >
>>> > > > > > Звать ngx_terminate_worker_processes() - это не очень хорошая
>>> > > > > > идея, это всё-таки аварийный механизм, и может приводить к
>>> > > > > > нехорошему. Тут имеет смысл как минимум попытаться штатно
>>> > > > > > завершить рабочие процессы.
>>> > > > > >
>>> > > > > > --
>>> > > > > > Maxim Dounin
>>> > > > > > http://nginx.org/en/donation.html
>>> > > > > >
>>> > > > > > _______________________________________________
>>> > > > > > nginx-ru mailing list
>>> > > > > > nginx-ru at nginx.org
>>> > > > > > http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>> > > >
>>> > > > > _______________________________________________
>>> > > > > nginx-ru mailing list
>>> > > > > nginx-ru at nginx.org
>>> > > > > http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>> > > >
>>> > > >
>>> > > > --
>>> > > > Maxim Dounin
>>> > > > http://nginx.org/en/donation.html
>>> > > >
>>> > > > _______________________________________________
>>> > > > nginx-ru mailing list
>>> > > > nginx-ru at nginx.org
>>> > > > http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>> > > >
>>> >
>>> > > _______________________________________________
>>> > > nginx-ru mailing list
>>> > > nginx-ru at nginx.org
>>> > > http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>> >
>>> >
>>> > --
>>> > Maxim Dounin
>>> > http://nginx.org/en/donation.html
>>> >
>>> > _______________________________________________
>>> > nginx-ru mailing list
>>> > nginx-ru at nginx.org
>>> > http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>> >
>>
>>> _______________________________________________
>>> nginx-ru mailing list
>>> nginx-ru at nginx.org
>>> http://mailman.nginx.org/mailman/listinfo/nginx-ru
>>
>>
>> --
>> Maxim Dounin
>> http://nginx.org/en/donation.html
>>
>> _______________________________________________
>> nginx-ru mailing list
>> nginx-ru at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-ru
Подробная информация о списке рассылки nginx-ru