httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@covalent.net>
Subject Re: [Patch]: Patch to make threaded mpm use join to cleanup workers.
Date Sat, 14 Apr 2001 20:08:25 GMT

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                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message