All in all the patch looks good, just a few comments. BTW, the reason the
join was removed a few years ago, is that we were doing it in
clean_child_exit, which meant it was happening during signal handling,
which is bad.
> if ((rv = apr_accept(&csd, sd, ptrans)) != APR_SUCCESS) {
> csd = NULL;
> - ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf,
> - "apr_accept");
> + ap_log_error(APLOG_MARK, APLOG_ERR, rv, ap_server_conf,
>"apr_accept");
Please remove any formating issues from the patch. They just clutter what
you are really doing here.
> - apr_threadattr_detach_set(thread_attr, 1);
> + apr_threadattr_detach_set(thread_attr, PTHREAD_CREATE_JOINABLE);
This is a big no-no. You can't use a PTHREAD macro in an apr_thread call.
> +
> + /* What state should this child_main process be listed as in the scoreboard...?
> + ap_update_child_status(my_child_num, i, SERVER_STARTING, (request_rec *) NULL);
> + */
Please fix these comments so that they don't wrap, and use the Apache
comment style.
> apr_signal_thread(check_signal);
> +
> + /* A terminating signal was received. Now join each of the workers to clean
>them up. */
> + /* If the worker already exitted, then the join frees their resources and
>returns. */
> + /* If the worker hasn't exited, then this blocks until they have (then cleans
>up). */
Same as above.
Other than those few comments, please commit.
Ryan
_______________________________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
