You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Gregg L. Smith" <gl...@gknw.net> on 2012/01/24 02:29:15 UTC
Re: svn commit: r1234169 - in /httpd/mod_fcgid/trunk/modules/fcgid:
Bill,
In PR 50309, the submitted patch moved the apr_pool_cleanup_register() call from procmgr_child_init to the tail end of procmgr_post_config in fcgid_pm_win.c prior to returning APR_SUCCESS. You have simply removed it from procmgr_child_init in r1234169.
https://issues.apache.org/bugzilla/attachment.cgi?id=27982&action=diff
I am a little worried that without registering a cleanup somewhere (where the patch proposes), there will be none. This patch has now been in use by Mario, Steffen of Apachelounge and myself now for well over a year, and we know it works as it was posted.
However, I will respect your knowledge if you say it is not needed. I just want to make sure it was not missed.
Regards,
Gregg
Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c
URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c?r \
ev=1234169&r1=1234168&r2=1234169&view=diff \
====================================================================== \
========
--- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c (original)
+++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c Fri Jan 20 22:02:50 2012
@@ -248,8 +248,8 @@ apr_status_t procmgr_peek_cmd(fcgid_comm
apr_status_t
procmgr_child_init(server_rec * main_server, apr_pool_t * pchild)
{
- apr_pool_cleanup_register(pchild, main_server,
- procmgr_stop_procmgr, apr_pool_cleanup_null);
+ /* Noop on windows, but used by unix */
+
return APR_SUCCESS;
}
Re: svn commit: r1234169 - in /httpd/mod_fcgid/trunk/modules/fcgid:
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 1/23/2012 9:14 PM, William A. Rowe Jr. wrote:
>
> The small change to this patch is to reflect that the absence of code
> from _child_init in no way allows us to eliminate our handling for the
> _child_init phase; it is required on windows.
Started at this too long... that phase "is required on Unix" . :)
Again, thanks for the sharp eyes!
Re: svn commit: r1234169 - in /httpd/mod_fcgid/trunk/modules/fcgid:
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
Greg, thanks for your comments...
On 1/23/2012 7:29 PM, Gregg L. Smith wrote:
> Bill,
>
> In PR 50309, the submitted patch moved the apr_pool_cleanup_register() call from procmgr_child_init to the tail end of procmgr_post_config in fcgid_pm_win.c prior to returning APR_SUCCESS. You have simply removed it from procmgr_child_init in r1234169.
>
> https://issues.apache.org/bugzilla/attachment.cgi?id=27982&action=diff
>
> I am a little worried that without registering a cleanup somewhere (where the patch proposes), there will be none.
The small change to this patch is to reflect that the absence of code
from _child_init in no way allows us to eliminate our handling for the
_child_init phase; it is required on windows.
> Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c
> URL: http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c?r \
> ev=1234169&r1=1234168&r2=1234169&view=diff \
> ====================================================================== \
> ========
> --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c (original)
> +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c Fri Jan 20 22:02:50 2012
> @@ -248,8 +248,8 @@ apr_status_t procmgr_peek_cmd(fcgid_comm
> apr_status_t
> procmgr_child_init(server_rec * main_server, apr_pool_t * pchild)
> {
> - apr_pool_cleanup_register(pchild, main_server,
> - procmgr_stop_procmgr, apr_pool_cleanup_null);
> + /* Noop on windows, but used by unix */
> +
> return APR_SUCCESS;
> }
Not certain what happened to the remainder of this patch, other than perhaps
some odd line-ending interaction(?), but I'm very glad you caught this!
Fixed now in r1235115.