You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/08/23 20:43:55 UTC

Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c


On 08/23/2007 02:10 AM, wrowe@apache.org wrote:
> Author: wrowe
> Date: Wed Aug 22 17:10:35 2007
> New Revision: 568779
> 
> URL: http://svn.apache.org/viewvc?rev=568779&view=rev
> Log:
> main core: Emit errors during the initial apr_app_initialize()
> or apr_pool_create() (when apr-based error reporting is not ready).

In general this looks correct, but could you please give me a pointer
what exactly fails with ap_log_error?
Why is it safe to use apr_ctime() and apr_time_now() in this case?
It would help me to better understand the problem and the reasoning.


Regards

Rüdiger


Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> On 08/23/2007 10:13 PM, Jim Jagielski wrote:
>> Yeah, the conditions and assumptions on which this
>> is based warrant some comments in the code :)
> 
> +1

Ready to commit to the trunk/ flavor, once we have svn pre-commit hook
resolved.  Joe is looking at this.

Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 08/23/2007 10:13 PM, Jim Jagielski wrote:
> 
> On Aug 23, 2007, at 4:05 PM, William A. Rowe, Jr. wrote:
> 
>> Ruediger Pluem wrote:
>>>
>>> But I admit that this is harder to audit and is more likely to change
>>> at some
>>> point of time to the usage of a pool.
>>
>> More to the point, implementation of apr_ctime.  The alternative of no
>> error
>> at all or no timestamp seemed worse, to me.  Maybe an XXX comment on
>> trunk
>> to that effect?  (I don't so much care about 0.9/1.2 which aren't moving
>> targets, like trunk).
> 
> Yeah, the conditions and assumptions on which this
> is based warrant some comments in the code :)

+1

Regards

Rüdiger


Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 23, 2007, at 4:05 PM, William A. Rowe, Jr. wrote:

> Ruediger Pluem wrote:
>>
>> But I admit that this is harder to audit and is more likely to  
>> change at some
>> point of time to the usage of a pool.
>
> More to the point, implementation of apr_ctime.  The alternative of  
> no error
> at all or no timestamp seemed worse, to me.  Maybe an XXX comment  
> on trunk
> to that effect?  (I don't so much care about 0.9/1.2 which aren't  
> moving
> targets, like trunk).

Yeah, the conditions and assumptions on which this
is based warrant some comments in the code :)


Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> But I admit that this is harder to audit and is more likely to change at some
> point of time to the usage of a pool.

More to the point, implementation of apr_ctime.  The alternative of no error
at all or no timestamp seemed worse, to me.  Maybe an XXX comment on trunk
to that effect?  (I don't so much care about 0.9/1.2 which aren't moving
targets, like trunk).

> So you will get my votes (currently there seems to be svn issue where the
> pre-commit hook always fails).

Ditto.  There's now an -r2.patch of the one you noted was incomplete for 2.0,
so you can look at that, even though I can't provide the new link in STATUS
right now.

Bill

Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 23, 2007, at 4:01 PM, Ruediger Pluem wrote:

>
>
> On 08/23/2007 09:29 PM, William A. Rowe, Jr. wrote:
>> Ruediger Pluem wrote:
>>> On 08/23/2007 02:10 AM, wrowe@apache.org wrote:
>>>> Author: wrowe
>>>> Date: Wed Aug 22 17:10:35 2007
>>>> New Revision: 568779
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=568779&view=rev
>>>> Log:
>>>> main core: Emit errors during the initial apr_app_initialize()
>>>> or apr_pool_create() (when apr-based error reporting is not ready).
>>> In general this looks correct, but could you please give me a  
>>> pointer
>>> what exactly fails with ap_log_error?
>>> Why is it safe to use apr_ctime() and apr_time_now() in this case?
>>> It would help me to better understand the problem and the reasoning.
>>
>> Presume apr is not initialized; it refuses to create our process- 
>> >pool,
>> or (in 2.0) the global NULL pool.
>
> Ok, got it. But if we would call ap_log_error immediately after we  
> detect a non
> APR_SUCCESS value of apr_app_initialize ap_log_error should work,  
> correct?
> Of course it can only work if it does not use pools (it seems to be  
> the case
> for me).
> But I admit that this is harder to audit and is more likely to  
> change at some
> point of time to the usage of a pool.

I'm looking to wrap my noggin around these changes
early tomorrow...



Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 08/23/2007 09:29 PM, William A. Rowe, Jr. wrote:
> Ruediger Pluem wrote:
>> On 08/23/2007 02:10 AM, wrowe@apache.org wrote:
>>> Author: wrowe
>>> Date: Wed Aug 22 17:10:35 2007
>>> New Revision: 568779
>>>
>>> URL: http://svn.apache.org/viewvc?rev=568779&view=rev
>>> Log:
>>> main core: Emit errors during the initial apr_app_initialize()
>>> or apr_pool_create() (when apr-based error reporting is not ready).
>> In general this looks correct, but could you please give me a pointer
>> what exactly fails with ap_log_error?
>> Why is it safe to use apr_ctime() and apr_time_now() in this case?
>> It would help me to better understand the problem and the reasoning.
> 
> Presume apr is not initialized; it refuses to create our process->pool,
> or (in 2.0) the global NULL pool.

Ok, got it. But if we would call ap_log_error immediately after we detect a non
APR_SUCCESS value of apr_app_initialize ap_log_error should work, correct?
Of course it can only work if it does not use pools (it seems to be the case
for me).
But I admit that this is harder to audit and is more likely to change at some
point of time to the usage of a pool.
So you will get my votes (currently there seems to be svn issue where the
pre-commit hook always fails).

Regards

Rüdiger


Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> On 08/23/2007 02:10 AM, wrowe@apache.org wrote:
>> Author: wrowe
>> Date: Wed Aug 22 17:10:35 2007
>> New Revision: 568779
>>
>> URL: http://svn.apache.org/viewvc?rev=568779&view=rev
>> Log:
>> main core: Emit errors during the initial apr_app_initialize()
>> or apr_pool_create() (when apr-based error reporting is not ready).
> 
> In general this looks correct, but could you please give me a pointer
> what exactly fails with ap_log_error?
> Why is it safe to use apr_ctime() and apr_time_now() in this case?
> It would help me to better understand the problem and the reasoning.

Presume apr is not initialized; it refuses to create our process->pool,
or (in 2.0) the global NULL pool.

apr_ctime and apr_time_now, *fortunately* do no pool allocation.

I was near ready to log this as "- [crit] bad thing happened", until
I audited ctime and time_now.

Yes, there's an implicit assumption that ctime and time_now won't end
up with a pool allocation.  Yes, there's near no likelyhood that this
would not function.  But on the off chance that mutexes or other APR
resources are too corrupt in the kernel to complete apr_init, we
should at least give the server a fighting chance to scream it's final
death.

Bill