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 Smith <gl...@gknw.net> on 2012/03/02 23:34:40 UTC

CHANGES-FCGID is incorrect

Hi,

First, http://svn.apache.org/viewvc?view=revision&revision=1236319

The patch in PR 51078 was never commited. However, I am -1 on committing 
this right now because I believe the root cause of this orphan described 
in PR 51078 is what has been fixed now by the commit of PR 50309. 
Without the PR 50309 fix I have run into this orphan after stopping 
httpd and having to go into processes and kill it manually.  It's the 
problem with graceful restarts, the fcgid process never got killed and 
it would stop the httpd parent because it was holding onto the port and 
logs. PR 50309 most likely will fix PR 48949 as well, since Mario 
describes the same issue on Ubuntu and the same patch worked for him on it.


Second, http://svn.apache.org/viewvc?view=revision&revision=1234178

http://svn.apache.org/viewvc?view=revision&revision=1213454
The patch applied is from PR 51020, so you have killed 2 PRs with the 
one stone, 50120 & 51520

The latter's a nit pick I know :)

Gregg

Re: CHANGES-FCGID is incorrect

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/16/2012 8:16 AM, Jeff Trawick wrote:
> On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr.
> <wr...@rowe-clan.net> wrote:
>> On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote:
>>>
>>> The patch does have value to a limited number of applications.  I even went
>>> as far as to put caviats in the docs, and a see-docs note to the directive
>>> cmd commentary.  I hope it dissuades the casual user from throwing it on there
>>> unless they know it won't corrupt their app and know it solves their bug.
>>>
>>> Please reconsider your veto.  I agree that it -should not- occur in any
>>> real world scenario.  But there are unreal scenarios of server configs
>>> and third party modules which spend minutes, not seconds, tearing down on
>>> shutdown.  Those are the bugs.  But users just want some help and I think
>>> this patch is some help in rare cases.  Disabled by default and won't be
>>> used by much of anyone, we hope.  I'll revert (yet again) if you really
>>> want to make a case that we should never support this, even as a workaround.
>>
>> Gregg, and Jeff and company, if this is reviewed and everyone is happy with
>> an end result, I'd like to ensure 2.4.2 compatibility and get tagging.
> 
> If you're out of bandwidth I can handle the T&R&Nag today or tomorrow,
> after doing some testing of 2.3.7-dev with httpd 2.4.3-dev (and double
> checking with 2.0 and 2.2).

I got to the same point... needing to look at this against 2.4.3-dev tree.
If you are happy, please don't hesitate to T&R!  Not sure if I could beat
you to a tag, but for sure I'll make several hours for testing and voting
here this week.  [I've had most of trunk/ in test for some time now.]




Re: CHANGES-FCGID is incorrect

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote:
>>
>> The patch does have value to a limited number of applications.  I even went
>> as far as to put caviats in the docs, and a see-docs note to the directive
>> cmd commentary.  I hope it dissuades the casual user from throwing it on there
>> unless they know it won't corrupt their app and know it solves their bug.
>>
>> Please reconsider your veto.  I agree that it -should not- occur in any
>> real world scenario.  But there are unreal scenarios of server configs
>> and third party modules which spend minutes, not seconds, tearing down on
>> shutdown.  Those are the bugs.  But users just want some help and I think
>> this patch is some help in rare cases.  Disabled by default and won't be
>> used by much of anyone, we hope.  I'll revert (yet again) if you really
>> want to make a case that we should never support this, even as a workaround.
>
> Gregg, and Jeff and company, if this is reviewed and everyone is happy with
> an end result, I'd like to ensure 2.4.2 compatibility and get tagging.

If you're out of bandwidth I can handle the T&R&Nag today or tomorrow,
after doing some testing of 2.3.7-dev with httpd 2.4.3-dev (and double
checking with 2.0 and 2.2).

>
> We seem well overdue for a release.
>
> There is a group of issues with spaces in command lines, spaces in file names
> that need to be resolved for the typical windows scenario (and obviously also
> unix, just more unusual there).  I think we are best off calling 2.3.7 about
> baked right now, tagging, reviewing and releasing, and calling 2.3.next the
> flavor which fixes these command/file string argument quirks.
>
>
>



-- 
Born in Roswell... married an alien...

Re: CHANGES-FCGID is incorrect

Posted by Gregg Smith <gl...@gknw.net>.
Bill, Jeff,

I agree 2.3.7 needs to be released for the stated reasons, I was +1 to 
that back a few weeks ago.

On 4/10/2012 9:11 AM, Jeff Trawick wrote:
> On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr.
> <wr...@rowe-clan.net>  wrote:
>> On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote:
>>> The patch does have value to a limited number of applications.  I even went
>>> as far as to put caviats in the docs, and a see-docs note to the directive
>>> cmd commentary.  I hope it dissuades the casual user from throwing it on there
>>> unless they know it won't corrupt their app and know it solves their bug.
>>>
>>> Please reconsider your veto.  I agree that it -should not- occur in any
>>> real world scenario.  But there are unreal scenarios of server configs
>>> and third party modules which spend minutes, not seconds, tearing down on
>>> shutdown.  Those are the bugs.  But users just want some help and I think
>>> this patch is some help in rare cases.  Disabled by default and won't be
>>> used by much of anyone, we hope.  I'll revert (yet again) if you really
>>> want to make a case that we should never support this, even as a workaround.
As far as my specific veto, I wouldn't call it a veto but more a concern 
as it's untested code vs. 50309 which is well tested, I'm trusting your 
knowledge here. Bill, your caveats in the docs about cleanup, being 
experimental and off by default is good enough for me. I should be able 
to build and do a little testing within the next 48 hours.

I'll have to look over the 50120 &51520 statement of mine hopefully 
within same time frame. I may have mixed up the number quit easily!


>> Gregg, and Jeff and company, if this is reviewed and everyone is happy with
>> an end result, I'd like to ensure 2.4.2 compatibility and get tagging.
> Gregg?
>
> My gut feel is that the feature will help in some real world
> circumstances, but I need to stare at Windows-specific code for a
> while to really understand the problem (where third-party modules can
> intervene, where a timeout comes into play, etc.).  At first glance it
> seems to simply be the case when apr_proc_kill(SIGKILL) doesn't work
> for whatever reason (third-party code, but not the usual code we worry
> about).
>
>> We seem well overdue for a release.
>>
>> There is a group of issues with spaces in command lines, spaces in file names
>> that need to be resolved for the typical windows scenario (and obviously also
>> unix, just more unusual there).  I think we are best off calling 2.3.7 about
>> baked right now, tagging, reviewing and releasing, and calling 2.3.next the
>> flavor which fixes these command/file string argument quirks.
> +1
>


Re: CHANGES-FCGID is incorrect

Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr.
<wr...@rowe-clan.net> wrote:
> On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote:
>>
>> The patch does have value to a limited number of applications.  I even went
>> as far as to put caviats in the docs, and a see-docs note to the directive
>> cmd commentary.  I hope it dissuades the casual user from throwing it on there
>> unless they know it won't corrupt their app and know it solves their bug.
>>
>> Please reconsider your veto.  I agree that it -should not- occur in any
>> real world scenario.  But there are unreal scenarios of server configs
>> and third party modules which spend minutes, not seconds, tearing down on
>> shutdown.  Those are the bugs.  But users just want some help and I think
>> this patch is some help in rare cases.  Disabled by default and won't be
>> used by much of anyone, we hope.  I'll revert (yet again) if you really
>> want to make a case that we should never support this, even as a workaround.
>
> Gregg, and Jeff and company, if this is reviewed and everyone is happy with
> an end result, I'd like to ensure 2.4.2 compatibility and get tagging.

Gregg?

My gut feel is that the feature will help in some real world
circumstances, but I need to stare at Windows-specific code for a
while to really understand the problem (where third-party modules can
intervene, where a timeout comes into play, etc.).  At first glance it
seems to simply be the case when apr_proc_kill(SIGKILL) doesn't work
for whatever reason (third-party code, but not the usual code we worry
about).

>
> We seem well overdue for a release.
>
> There is a group of issues with spaces in command lines, spaces in file names
> that need to be resolved for the typical windows scenario (and obviously also
> unix, just more unusual there).  I think we are best off calling 2.3.7 about
> baked right now, tagging, reviewing and releasing, and calling 2.3.next the
> flavor which fixes these command/file string argument quirks.

+1

Re: CHANGES-FCGID is incorrect

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote:
> 
> The patch does have value to a limited number of applications.  I even went
> as far as to put caviats in the docs, and a see-docs note to the directive
> cmd commentary.  I hope it dissuades the casual user from throwing it on there
> unless they know it won't corrupt their app and know it solves their bug.
> 
> Please reconsider your veto.  I agree that it -should not- occur in any
> real world scenario.  But there are unreal scenarios of server configs
> and third party modules which spend minutes, not seconds, tearing down on
> shutdown.  Those are the bugs.  But users just want some help and I think
> this patch is some help in rare cases.  Disabled by default and won't be
> used by much of anyone, we hope.  I'll revert (yet again) if you really
> want to make a case that we should never support this, even as a workaround.

Gregg, and Jeff and company, if this is reviewed and everyone is happy with
an end result, I'd like to ensure 2.4.2 compatibility and get tagging.

We seem well overdue for a release.

There is a group of issues with spaces in command lines, spaces in file names
that need to be resolved for the typical windows scenario (and obviously also
unix, just more unusual there).  I think we are best off calling 2.3.7 about
baked right now, tagging, reviewing and releasing, and calling 2.3.next the
flavor which fixes these command/file string argument quirks.




Re: CHANGES-FCGID is incorrect

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
I have corrected the 1236319 entry which should not have been recorded yet.

The patch does have value to a limited number of applications.  I even went
as far as to put caviats in the docs, and a see-docs note to the directive
cmd commentary.  I hope it dissuades the casual user from throwing it on there
unless they know it won't corrupt their app and know it solves their bug.

Please reconsider your veto.  I agree that it -should not- occur in any
real world scenario.  But there are unreal scenarios of server configs
and third party modules which spend minutes, not seconds, tearing down on
shutdown.  Those are the bugs.  But users just want some help and I think
this patch is some help in rare cases.  Disabled by default and won't be
used by much of anyone, we hope.  I'll revert (yet again) if you really
want to make a case that we should never support this, even as a workaround.

I presume you meant 51560 borrows the 51020 patch.  Please recheck, we might
be working a record for the number of transpositions in one bug resolution.

On 3/2/2012 6:57 PM, William A. Rowe Jr. wrote:
> Gregg,
> 
> I reached the same conclusion Wednesday when I stumbled on a dirty mod_fcgid
> checkout here.
> 
> I'll review your comments and then determine how to proceed (commit the feature
> which could be useful, port the feature to apr, or concur it isn't needed.)
> 
> Because of the way the Windows Service Control manager works, it may still have
> value.
> 
> On 3/2/2012 4:34 PM, Gregg Smith wrote:
>> Hi,
>>
>> First, http://svn.apache.org/viewvc?view=revision&revision=1236319
>>
>> The patch in PR 51078 was never commited. However, I am -1 on committing this right now
>> because I believe the root cause of this orphan described in PR 51078 is what has been
>> fixed now by the commit of PR 50309. Without the PR 50309 fix I have run into this orphan
>> after stopping httpd and having to go into processes and kill it manually.  It's the
>> problem with graceful restarts, the fcgid process never got killed and it would stop the
>> httpd parent because it was holding onto the port and logs. PR 50309 most likely will fix
>> PR 48949 as well, since Mario describes the same issue on Ubuntu and the same patch worked
>> for him on it.
>>
>>
>> Second, http://svn.apache.org/viewvc?view=revision&revision=1234178
>>
>> http://svn.apache.org/viewvc?view=revision&revision=1213454
>> The patch applied is from PR 51020, so you have killed 2 PRs with the one stone, 50120 &
>> 51520
>>
>> The latter's a nit pick I know :)
>>
>> Gregg
>>
> 
> 


Re: CHANGES-FCGID is incorrect

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
Gregg,

I reached the same conclusion Wednesday when I stumbled on a dirty mod_fcgid
checkout here.

I'll review your comments and then determine how to proceed (commit the feature
which could be useful, port the feature to apr, or concur it isn't needed.)

Because of the way the Windows Service Control manager works, it may still have
value.

On 3/2/2012 4:34 PM, Gregg Smith wrote:
> Hi,
> 
> First, http://svn.apache.org/viewvc?view=revision&revision=1236319
> 
> The patch in PR 51078 was never commited. However, I am -1 on committing this right now
> because I believe the root cause of this orphan described in PR 51078 is what has been
> fixed now by the commit of PR 50309. Without the PR 50309 fix I have run into this orphan
> after stopping httpd and having to go into processes and kill it manually.  It's the
> problem with graceful restarts, the fcgid process never got killed and it would stop the
> httpd parent because it was holding onto the port and logs. PR 50309 most likely will fix
> PR 48949 as well, since Mario describes the same issue on Ubuntu and the same patch worked
> for him on it.
> 
> 
> Second, http://svn.apache.org/viewvc?view=revision&revision=1234178
> 
> http://svn.apache.org/viewvc?view=revision&revision=1213454
> The patch applied is from PR 51020, so you have killed 2 PRs with the one stone, 50120 &
> 51520
> 
> The latter's a nit pick I know :)
> 
> Gregg
>