[nginx] unnormal pid file

[洪志道]

Hi.

diff -r 4d874b4d82ed src/core/ngx_cycle.c
--- a/src/core/ngx_cycle.c Thu Mar 16 20:38:31 2017 +0300
+++ b/src/core/ngx_cycle.c Fri Mar 17 22:28:54 2017 -0400
@@ -979,39 +979,17 @@
 }


-void
-ngx_delete_pidfile(ngx_cycle_t *cycle)
-{
-    u_char           *name;
-    ngx_core_conf_t  *ccf;
-
-    ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
ngx_core_module);
-
-    name = ngx_new_binary ? ccf->oldpid.data : ccf->pid.data;
-
-    if (ngx_delete_file(name) == NGX_FILE_ERROR) {
-        ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
-                      ngx_delete_file_n " \"%s\" failed", name);
-    }
-}
-
-
-ngx_int_t
-ngx_signal_process(ngx_cycle_t *cycle, char *sig)
+static ngx_pid_t
+ngx_get_filepid(ngx_cycle_t *cycle, ngx_str_t *name)
 {
     ssize_t           n;
     ngx_pid_t         pid;
     ngx_file_t        file;
-    ngx_core_conf_t  *ccf;
     u_char            buf[NGX_INT64_LEN + 2];

-    ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "signal process started");
-
-    ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
ngx_core_module);
-
     ngx_memzero(&file, sizeof(ngx_file_t));

-    file.name = ccf->pid;
+    file.name = *name;
     file.log = cycle->log;

     file.fd = ngx_open_file(file.name.data, NGX_FILE_RDONLY,
@@ -1020,7 +998,7 @@
     if (file.fd == NGX_INVALID_FILE) {
         ngx_log_error(NGX_LOG_ERR, cycle->log, ngx_errno,
                       ngx_open_file_n " \"%s\" failed", file.name.data);
-        return 1;
+        return -1;
     }

     n = ngx_read_file(&file, buf, NGX_INT64_LEN + 2, 0);
@@ -1031,7 +1009,7 @@
     }

     if (n == NGX_ERROR) {
-        return 1;
+        return -1;
     }

     while (n-- && (buf[n] == CR || buf[n] == LF)) { /* void */ }
@@ -1042,6 +1020,55 @@
         ngx_log_error(NGX_LOG_ERR, cycle->log, 0,
                       "invalid PID number \"%*s\" in \"%s\"",
                       n, buf, file.name.data);
+        return -1;
+    }
+
+    return pid;
+}
+
+
+void
+ngx_delete_pidfile(ngx_cycle_t *cycle)
+{
+    ngx_pid_t         pid;
+    ngx_str_t        *name;
+    ngx_core_conf_t  *ccf;
+
+    ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
ngx_core_module);
+
+    name = ngx_new_binary ? &ccf->oldpid : &ccf->pid;
+
+    pid = ngx_get_filepid(cycle, name);
+    if (pid == -1) {
+        return;
+    }
+
+    if (pid != ngx_pid) {
+        ngx_log_error(NGX_LOG_ALERT, cycle->log, 0, "pidfile has been
changed %P %P",
+                      pid, ngx_pid);
+        return;
+    }
+
+    if (ngx_delete_file(name) == NGX_FILE_ERROR) {
+        ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
+                      ngx_delete_file_n " \"%s\" failed", name->data);
+    }
+}
+
+
+ngx_int_t
+ngx_signal_process(ngx_cycle_t *cycle, char *sig)
+{
+    ngx_pid_t         pid;
+    ngx_core_conf_t  *ccf;
+
+    ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "signal process started");
+
+    ccf = (ngx_core_conf_t *) ngx_get_conf(cycle->conf_ctx,
ngx_core_module);
+
+    pid = ngx_get_filepid(cycle, &ccf->pid);
+
+    if (pid == -1) {
         return 1;
     }

Thanks.


2017-03-18 15:43 GMT+08:00 洪志道 <hongzhidao at gmail.com>:

> Hi.
>
> It's easy to reproduce such problem that
> the master/worker process are running without the pid file.
>
> Step by step:
>
> 1. Start master process.
>     > ./objs/nginx
>     nginx.pid successfully create with content (23234)
>
> 2. Create an active connection, purpose for prevent previous master exit.
>    > telnet 127.1 80
>
> 3. Quit the previous master, and it still alive for a while.
>    > kill -QUIT 23234
>
> 4. Start new master process 23361, and the content of nginx.pid is changed
> to 23361
>    > ./objs/nginx
>    It successfully start because of the listening port is closed by the
> step 3.
>
> 5. Wait for a while, and the nginx.pid is deleted while the old master
> process (23234) completely quit.
>
> Now the master process (23361) is running without nginx.pid.
>
> So, there are two key points:
> 1. Master start, I think there is not problem.
> 2. Master Exit, is it better to add the pid check with the pid file?
>
> Anyway, I think we should guarantee the pid file associates with the
> running process.
>
> src/os/unix/ngx_process_cycle.c
>
> +static ngx_int_t
> +ngx_is_same_pid(ngx_cycle_t *cycle)
> +{
> +        return read_pid_from_pidfile. // TODO
> +}
>
> static void
> ngx_master_process_exit(ngx_cycle_t *cycle)
> {
>     ngx_uint_t  i;
>
> -   ngx_delete_pidfile(cycle);
> +  if (ngx_is_same_pid(cycle)) {
> +        ngx_delete_pidfile(cycle);
> +
> +  } else {
> +        ngx_log_error(NGX_LOG_NOTICE, cycle->log, 0, "blalala...");
> + }
>
> Thanks.
> B.R.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20170318/d8f2ec18/attachment.html>