Re: патч для nginx/win32
Maxim Dounin
mdounin at mdounin.ru
Wed Mar 27 13:38:53 UTC 2013
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