httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r543511 - /httpd/httpd/branches/1.3.x/src/main/http_main.c
Date Fri, 01 Jun 2007 21:30:22 GMT


On 06/01/2007 05:42 PM, jim@apache.org wrote:
> Author: jim
> Date: Fri Jun  1 08:42:57 2007
> New Revision: 543511
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=543511
> Log:
> Add in parent process PID table, to provide for
> a check against the pid values located in the
> scoreboard.
> 
> Modified:
>     httpd/httpd/branches/1.3.x/src/main/http_main.c
> 
> Modified: httpd/httpd/branches/1.3.x/src/main/http_main.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/1.3.x/src/main/http_main.c?view=diff&rev=543511&r1=543510&r2=543511
> ==============================================================================
> --- httpd/httpd/branches/1.3.x/src/main/http_main.c (original)
> +++ httpd/httpd/branches/1.3.x/src/main/http_main.c Fri Jun  1 08:42:57 2007
> @@ -354,9 +354,17 @@
>  char tpf_mutex_key[TPF_MUTEX_KEY_SIZE];
>  #endif /* TPF */
>  
> +/*
> + * Shared memory scoreboard
> + */
>  scoreboard *ap_scoreboard_image = NULL;
>  
>  /*
> + * Parent process local storage of child pids
> + */
> +static table *pid_table;

For my understanding (and a bit of devils advocate here :-)): Why do we use a
table here and not a fixed size array (HARD_SERVER_LIMIT) of ints (apr_array of
pid_t in the 2.x case). If we keep the pids at the same index as in the
scoreboard the checks would be somewhat faster and simpler to do. Of course
we waste more memory.


> @@ -2787,6 +2821,11 @@
>  	    if (pid == my_pid || pid == 0)
>  		continue;
>  
> +            if (!in_pid_table(pid)) {
> +                ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
> +                             "Bad pid (%d) in scoreboard slot %d", pid, i);
> +                continue;
> +            }
>  	    waitret = waitpid(pid, &status, WNOHANG);
>  	    if (waitret == pid || waitret == -1) {
>  		ap_scoreboard_image->parent[i].pid = 0;

Don't we need to remove the pid from our local table here as well?


> @@ -5113,8 +5170,15 @@
>  		else if (ps->last_rtime + ss->timeout_len < now) {
>  		    /* no progress, and the timeout length has been exceeded */
>  		    ss->timeout_len = 0;
> -		    kill(ps->pid, SIG_TIMEOUT_KILL);
> -		}
> +                    pid = ps->pid;
> +                    if (in_pid_table(pid)) {
> +                        kill(pid, SIG_TIMEOUT_KILL);

Don't we need to remove the pid from our local table here as well?

> +                    }
> +                    else {
> +                        ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, server_conf,
> +                            "Bad pid (%d) in scoreboard slot %d", pid, i);
> +                    }
> +                }
>  	    }
>  #endif
>  	}

Regards

RĂ¼diger


Mime
View raw message