You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2007/08/23 02:10:35 UTC
svn commit: r568779 - /httpd/httpd/trunk/server/main.c
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).
This moves apr_app_initialize() into init_process (and indirects the
argv/argc parameters for this function) since the same error logging
is appropriate to either failure.
Note the change of the internal name create_process to init_process,
since create_process means something very different in apr-land.
(Replaces the misapplied commit r568762, already backed out).
Modified:
httpd/httpd/trunk/server/main.c
Modified: httpd/httpd/trunk/server/main.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/main.c?rev=568779&r1=568778&r2=568779&view=diff
==============================================================================
--- httpd/httpd/trunk/server/main.c (original)
+++ httpd/httpd/trunk/server/main.c Wed Aug 22 17:10:35 2007
@@ -20,6 +20,7 @@
#include "apr_general.h"
#include "apr_lib.h"
#include "apr_md5.h"
+#include "apr_time.h"
#include "apr_version.h"
#include "apu_version.h"
@@ -276,20 +277,29 @@
return retcode; /* unreachable, hopefully. */
}
-static process_rec *create_process(int argc, const char * const *argv)
+static process_rec *init_process(int *argc, const char * const * *argv)
{
process_rec *process;
apr_pool_t *cntx;
apr_status_t stat;
+ const char *failed = "apr_app_initialize()";
+ stat = apr_app_initialize(argc, argv, NULL);
+ if (stat == APR_SUCCESS) {
+ failed = "apr_pool_create()";
+ stat = apr_pool_create(&cntx, NULL);
+ }
- stat = apr_pool_create(&cntx, NULL);
if (stat != APR_SUCCESS) {
- /* XXX From the time that we took away the NULL pool->malloc mapping
- * we have been unable to log here without segfaulting.
+ /* For all intents and purposes, this is impossibly unlikely,
+ * but APR doesn't exist yet, we can't use it for reporting
+ * these earliest two failures;
*/
- ap_log_error(APLOG_MARK, APLOG_ERR, stat, NULL,
- "apr_pool_create() failed to create "
- "initial context");
+ char ctimebuff[APR_CTIME_LEN + 1];
+ apr_ctime(ctimebuff, apr_time_now());
+ ctimebuff[APR_CTIME_LEN] = '\0';
+ fprintf(stderr, "[%s] [crit] (%d) %s: %s failed "
+ "to initial context, exiting",
+ ctimebuff, stat, (*argv)[0], failed);
apr_terminate();
exit(1);
}
@@ -298,14 +308,19 @@
apr_pool_tag(cntx, "process");
ap_open_stderr_log(cntx);
+ /* Now we have initialized apr and our logger, no more
+ * exceptional error reporting required for the lifetime
+ * of this server process.
+ */
+
process = apr_palloc(cntx, sizeof(process_rec));
process->pool = cntx;
apr_pool_create(&process->pconf, process->pool);
apr_pool_tag(process->pconf, "pconf");
- process->argc = argc;
- process->argv = argv;
- process->short_name = apr_filepath_name_get(argv[0]);
+ process->argc = *argc;
+ process->argv = *argv;
+ process->short_name = apr_filepath_name_get((*argv)[0]);
return process;
}
@@ -458,9 +473,7 @@
AP_MONCONTROL(0); /* turn off profiling of startup */
- apr_app_initialize(&argc, &argv, NULL);
-
- process = create_process(argc, argv);
+ process = init_process(&argc, &argv);
pglobal = process->pool;
pconf = process->pconf;
ap_server_argv0 = process->short_name;
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
Re: svn commit: r568779 - /httpd/httpd/trunk/server/main.c
Posted by Ruediger Pluem <rp...@apache.org>.
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