You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 1999/11/23 22:56:32 UTC
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
On 23 Nov 1999 rbb@hyperreal.org wrote:
>...
> --- apr_execve.c 1999/11/22 18:01:14 1.4
> +++ apr_execve.c 1999/11/23 21:22:45 1.5
> @@ -349,7 +349,6 @@
> newargv = (char **) malloc((p - lbuf + 1)
> + (i + sargc + 1) * sizeof(*newargv));
> if (newargv == NULL) {
> - fprintf(stderr, "Ouch! Out of memory in hashbang()!\n");
> return NULL;
Do callers check this return code? Should we have an abort() here?
>...
> --- apr_pools.c 1999/10/29 21:52:32 1.20
> +++ apr_pools.c 1999/11/23 21:22:47 1.21
> @@ -268,8 +268,7 @@
>
> blok = (union block_hdr *) malloc(size + sizeof(union block_hdr));
> if (blok == NULL) {
> - fprintf(stderr, "Ouch! malloc failed in malloc_block()\n");
> - exit(1);
> + return NULL;
This doesn't look good! A caller might get a NULL pointer now, whereas
before they were guaranteed not to (cuz the program would dump rather than
doing so).
>...
> @@ -299,7 +298,6 @@
> "at the end of a block!\n");
> while (free_blk) {
> if (free_blk == blok) {
> - fprintf(stderr, "Ouch! Freeing free block\n");
> abort();
> exit(1);
I still think we need a platform-dependent way to log this kind of error
message. Apache should NOT simply exit without saying anything. The admin
could be scratching their head for weeks until they figure out "oh! maybe
it is running out of memory!"
-0 on abort() with no message.
(this applies to the other places where you removed a message before an
abort())
>...
> --- apr_tables.c 1999/10/17 16:11:43 1.5
> +++ apr_tables.c 1999/11/23 21:22:48 1.6
> @@ -284,9 +284,7 @@
> static ap_table_entry_t *table_push(ap_table_t *t)
> {
> if (t->a.nelts == t->a.nalloc) {
> - fprintf(stderr,
> - "table_push: ap_table_t created by %p hit limit of %u\n",
> - t->creator, t->a.nalloc);
> + return NULL;
> }
> return (ap_table_entry_t *) ap_push_array(&t->a);
In the old behavior, the caller would always get memory back. This is no
longer the case. We should either abort (with a message!) or go ahead and
log the problem, but return the memory anyhow.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, 24 Nov 1999, Greg Marr wrote:
>...
> >By "talking Apache here", I meant "Apache-using-APR". In that
> >condition,
> >everything ever written for Apache assumes that a memory allocation
> >will
> >SUCCEED. So, in light of Apache... yes, we should be killing the
> >process
> >if we can't get the memory.
>
> If that's what Apache wants to do, then Apache should kill the
> process when a malloc fails, not APR.
With the hook approach, this is reasonable to do. I do not see any other
way that would be convenient.
>...
> >And I say no. The current model of "alloc and you'll get it" means
> >that Apache can be very fast. It doesn't have to worry about not
> >getting memory.
>
> Yes it does, just in a different place. Something has to decide that
> Apache should log the error and die. You're saying that APR is that
> place. Ryan is saying that Apache is that place. In either case,
> someone has to do it. In order for APR to be usable for anything
> outside of Apache, that place needs to be Apache.
If you do it at the bottom, then you check once and die. Nobody else has
to ever check. I'm against the model where you have to check the status
code all the way back up the stack.
> >If you *do* have to worry about it, then you start putting checks on
> >every darn function call.
>
> This is a bad thing?
Certainly.
I don't have to do it now. It is a significant, incremental effort to add
these checks throughout my code. That is a bad thing.
No, the checks in-and-of themselves are not "bad", but I believe the
current programming model in Apache is much better. A developer doesn't
have to worry about a malloc failure, and (unrelated to this particular
thread) they don't have to worry about freeing resources (and various ways
to cleanup on abnormal function exit).
> >You ever see what that looks like? Go look at some COM code in
> >Windows. You have one line of work, three lines of error
> >handling. It is absolutely horrible.
>
> I program in Windows for a living.
I used to.
> We use a library that used to like to abort when something went. It
> was a royal pain. We finally convinced them that aborting was a bad
> thing, since our customers see it as an error on our part.
For a library like that... yes, it is a bad thing. Never said that
libraries should do this. I'm saying *Apache* should abort on memory alloc
failure. If the lowest point in the calltree happens to be APR, then it
should be doing that for us.
> >Further, the time to check a result, when it is typically successful
> >is just wasted time.
>
> You are kidding, right? Please tell me you're kidding.
No, I'm not. Yah... individual checks are cheap. Keep doing it
*everywhere* and they add up. Realize that it is more than just a check:
typically you have storage into a value (for propagating that error out).
If these checks occur within a loop, then again: they add up.
Sure, a percent or two. But that matters to some people :-)
I'm more against the checks for aesthetic purposes, than the performance
problem. But I had to mention it :-)
> >Lastly, people will just start getting lazy and not putting in
> >checks. Then you end up with a case where a NULL pointer gets hit
> >at some arbitrary point in the code, a long ways away from the
> >(failed) allocation. Tracking that back is a bitch.
>
> That is why you need to check return codes.
"need" is a great ideal. But that isn't how things truly work. People
*will* skip checks.
> >Changing return codes is one thing. Changing the basic programming
> >model and semantics of Apache development is something else entirely.
>
> That is why it needs to be done as few times as possible. Get all of
> the changes in at once.
If the decision to alter the basic model is accepted in the first place --
yes, do it now rather than later.
> >I could be mistaken on Ryan's intent (and I asked him that in my
> >last email), but if we are really talking about needing to check a
> >status code from ap_pstrup(), then I'll be quite upset.
>
> You mean you don't check return codes now?
Of course not! ap_pstrdup() does NOT have a return code! That's my point
here. Take the following line of code:
ap_log_rerror(...,
ap_psprintf(r->pool, "error at URL \"%s\".",
ap_escape_html(r->pool, r->uri)));
Unwind that sucker and put in checks. Ack! No thank you. What is currently
a very clean piece of code would become wordy and obnoxious.
>...
> >Sure, APR is a library, but it is still an integral part of Apache
> >and one of its goals is to be well-tuned for Apache's needs.
>
> Well tuned, yes. Pigeon-holed into, no.
Programming model change, no.
> >And yes, I think a malloc should bring things down. It is a better
> >model than boatloads of memory-status checks.
>
> I don't agree. It may not be as critical in Apache's case, but if my
> company's main product exited when a malloc failed, then customers
> would lose data.
Well, come on. Of course. I'm not suggesting that this style is used
outside of Apache.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c
apr_pools.c apr_tables.c
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 07:45 AM 11/24/99 , Greg Stein wrote:
>On Wed, 24 Nov 1999, Greg Marr wrote:
> > At 03:03 AM 11/24/99 , Greg Stein wrote:
> > >On Tue, 23 Nov 1999, Ryan Bloom wrote:
> > > > libraries shouldn't be killing off processes because there is
> a problem,
> > > > they should alert the program that there is a problem. If we
> aren't
> > > > checking the return code, we should be. That will be fixed
> in the next
> > > > few patches.
> > >
> > >That's a cop-out. We are talking Apache here.
> >
> > No, we are talking APR here. APR is not Apache.
>
>By "talking Apache here", I meant "Apache-using-APR". In that
>condition,
>everything ever written for Apache assumes that a memory allocation
>will
>SUCCEED. So, in light of Apache... yes, we should be killing the
>process
>if we can't get the memory.
If that's what Apache wants to do, then Apache should kill the
process when a malloc fails, not APR.
>I also maintain that we must at least attempt to log the error
>somewhere, rather than silently die.
No one is disagreeing with you on that point. You are correct,
Apache should log the error before Apache kills itself.
> > >All throughout Apache, we allocate memory with the presumption
> that
> > >if the function returns, then we *have* allocated that memory.
> > >
> > >You simply CANNOT change those semantics right now.
> >
> > Hey, if he wants to change the semantics, which involves adding
> > return value checks to all the calls, then I say go ahead. If you
> > REALLY need to have a function you can call from within Apache
> that
> > aborts if memory isn't available, then add a wrapper function *in
> > Apache* that calls APR and aborts if memory isn't available.
>
>And I say no. The current model of "alloc and you'll get it" means
>that Apache can be very fast. It doesn't have to worry about not
>getting memory.
Yes it does, just in a different place. Something has to decide that
Apache should log the error and die. You're saying that APR is that
place. Ryan is saying that Apache is that place. In either case,
someone has to do it. In order for APR to be usable for anything
outside of Apache, that place needs to be Apache.
>If you *do* have to worry about it, then you start putting checks on
>every darn function call.
This is a bad thing?
>You ever see what that looks like? Go look at some COM code in
>Windows. You have one line of work, three lines of error
>handling. It is absolutely horrible.
I program in Windows for a living.
We use a library that used to like to abort when something went. It
was a royal pain. We finally convinced them that aborting was a bad
thing, since our customers see it as an error on our part.
>Further, the time to check a result, when it is typically successful
>is just wasted time.
You are kidding, right? Please tell me you're kidding.
>Lastly, people will just start getting lazy and not putting in
>checks. Then you end up with a case where a NULL pointer gets hit
>at some arbitrary point in the code, a long ways away from the
>(failed) allocation. Tracking that back is a bitch.
That is why you need to check return codes.
>Changing return codes is one thing. Changing the basic programming
>model and semantics of Apache development is something else entirely.
That is why it needs to be done as few times as possible. Get all of
the changes in at once.
>I could be mistaken on Ryan's intent (and I asked him that in my
>last email), but if we are really talking about needing to check a
>status code from ap_pstrup(), then I'll be quite upset.
You mean you don't check return codes now?
>If Apache is going to provide abort-on-malloc-failure wrappers, then
>that is somewhat acceptable. But there would be a lot of wrappers,
>and a lot of time cycling through those things.
Right. Therefore, it is much better to just check your return codes,
and let the particular module/function in question determine whether
or not a failed malloc is an error that should bring down the entire
server.
>Apache has its own logging mechanisms that it can use at its level.
>I'm talking about a "last resort" logging mechanism that APR can use.
APR can provide a last resort logging mechanism, but it shouldn't
kill the program that is using APR.
>Sure, APR is a library, but it is still an integral part of Apache
>and one of its goals is to be well-tuned for Apache's needs.
Well tuned, yes. Pigeon-holed into, no.
>And yes, I think a malloc should bring things down. It is a better
>model than boatloads of memory-status checks.
I don't agree. It may not be as critical in Apache's case, but if my
company's main product exited when a malloc failed, then customers
would lose data.
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
Greg,
The changes are going to require checking error codes. I don't have the
energy or desire to argue this one back and forth forever. If you are
going to veto any changes that require checking an error code, then back
out the change. Regardless, the ap_log_message call will not work
properly for reasons I have outlined in my previous messages. I would
rather just see fprintf(stderr in APR than see ap_log_message that people
will mis-use. I will veto an ap_log_message function.
Ryan
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c apr_tables.c
Posted by Manoj Kasichainula <ma...@io.com>.
I should've paid more attention to my Thanksgiving backlog.
> > How about using something like the C++ method where you can
> > register an out of memory handler function (set_new_handler())?
On Wed, Nov 24, 1999 at 08:03:18AM -0500, Ryan Bloom wrote:
> I could live with that
This is in fact what buff does right now. See ap_bonerror().
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
I like this. I may clean it up a bit before I commit it, and I'll have to
write a small little function to set the function pointer. Expect to see
it later today or tomorrow sometime (I'll have a lot of time in the
airport and on the plane tomorrow morning to code).
Greg, it's good to know you are out there making my life a royal PITA,
because everytime you jump on me for doing something, we come up with a
bit more flexible code. :-)
Ryan
On Wed, 24 Nov 1999, Greg Stein wrote:
> On Wed, 24 Nov 1999, Ryan Bloom wrote:
> > > How about using something like the C++ method where you can register an out
> > > of memory handler function (set_new_handler())? That way Apache can register
> > > a handler that does the log & abort step without forcing all APR using apps
> > > to have the same behaviour.
> >
> > I could live with that, presuming that the APR default is to jut return an
> > error code. If Apache wants to over-write that default, then fine. I
> > won't get to this code until after Thanksgiving, because I want to get
> > everybodies reaction first.
>
> You could probably do something like this:
>
> #define ap_memory_error(ctx) ((ctx) != NULL \
> && (ctx)->notify_no_memory != NULL \
> ? (*(ctx)->notify_no_memory)(ctx) \
> : APR_ENOMEMORY)
>
> (of course, it could be a real function, too)
>
> Code would then look like:
>
> p = malloc(foo);
> if (p == NULL)
> return ap_memory_error(ctx);
> p->bar = baz;
> ...
>
>
> Seem reasonable?
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, 24 Nov 1999, Ryan Bloom wrote:
> > How about using something like the C++ method where you can register an out
> > of memory handler function (set_new_handler())? That way Apache can register
> > a handler that does the log & abort step without forcing all APR using apps
> > to have the same behaviour.
>
> I could live with that, presuming that the APR default is to jut return an
> error code. If Apache wants to over-write that default, then fine. I
> won't get to this code until after Thanksgiving, because I want to get
> everybodies reaction first.
You could probably do something like this:
#define ap_memory_error(ctx) ((ctx) != NULL \
&& (ctx)->notify_no_memory != NULL \
? (*(ctx)->notify_no_memory)(ctx) \
: APR_ENOMEMORY)
(of course, it could be a real function, too)
Code would then look like:
p = malloc(foo);
if (p == NULL)
return ap_memory_error(ctx);
p->bar = baz;
...
Seem reasonable?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c apr_tables.c
Posted by Brian Havard <br...@kheldar.apana.org.au>.
On Wed, 24 Nov 1999 08:03:18 -0500 (EST), Ryan Bloom wrote:
>
>> How about using something like the C++ method where you can register an out
>> of memory handler function (set_new_handler())? That way Apache can register
>> a handler that does the log & abort step without forcing all APR using apps
>> to have the same behaviour.
>
>I could live with that, presuming that the APR default is to jut return an
>error code. If Apache wants to over-write that default, then fine. I
>won't get to this code until after Thanksgiving, because I want to get
>everybodies reaction first.
Yeah, that's what I was thinking. The best of both worlds. No return code
checking needed and Apache decides what to do in the event of a failure.
On Wed, 24 Nov 1999 05:09:06 -0800 (PST), Greg Stein wrote:
>Yes, I suggested something similar when I talked about a flag in the
>context. In that case, the flag meant "abort if there's no memory."
>
>Your callback approach is much cleaner and would do the same thing.
>
>And yes: if there was a callback from APR, then I'd be happy as long as
>Apache's callback aborted the process.
Good, everyone's happy :)
So can we rewrite APR in C++ now? (*ducks*).
--
______________________________________________________________________________
| Brian Havard | "He is not the messiah! |
| brianh@kheldar.apana.org.au | He's a very naughty boy!" - Life of Brian |
------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> How about using something like the C++ method where you can register an out
> of memory handler function (set_new_handler())? That way Apache can register
> a handler that does the log & abort step without forcing all APR using apps
> to have the same behaviour.
I could live with that, presuming that the APR default is to jut return an
error code. If Apache wants to over-write that default, then fine. I
won't get to this code until after Thanksgiving, because I want to get
everybodies reaction first.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Thu, 25 Nov 1999, Brian Havard wrote:
>...
> How about using something like the C++ method where you can register an out
> of memory handler function (set_new_handler())? That way Apache can register
> a handler that does the log & abort step without forcing all APR using apps
> to have the same behaviour.
Yes, I suggested something similar when I talked about a flag in the
context. In that case, the flag meant "abort if there's no memory."
Your callback approach is much cleaner and would do the same thing.
And yes: if there was a callback from APR, then I'd be happy as long as
Apache's callback aborted the process.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c apr_tables.c
Posted by Brian Havard <br...@kheldar.apana.org.au>.
On Wed, 24 Nov 1999 04:45:44 -0800 (PST), Greg Stein wrote:
>By "talking Apache here", I meant "Apache-using-APR". In that condition,
>everything ever written for Apache assumes that a memory allocation will
>SUCCEED. So, in light of Apache... yes, we should be killing the process
>if we can't get the memory.
>
>I also maintain that we must at least attempt to log the error somewhere,
>rather than silently die.
[...]
>And I say no. The current model of "alloc and you'll get it" means that
>Apache can be very fast. It doesn't have to worry about not getting
>memory.
>
>If you *do* have to worry about it, then you start putting checks on every
>darn function call. You ever see what that looks like? Go look at some COM
>code in Windows. You have one line of work, three lines of error handling.
>It is absolutely horrible. Further, the time to check a result, when it is
>typically successful is just wasted time. Lastly, people will just start
>getting lazy and not putting in checks. Then you end up with a case where
>a NULL pointer gets hit at some arbitrary point in the code, a long ways
>away from the (failed) allocation. Tracking that back is a bitch.
How about using something like the C++ method where you can register an out
of memory handler function (set_new_handler())? That way Apache can register
a handler that does the log & abort step without forcing all APR using apps
to have the same behaviour.
--
______________________________________________________________________________
| Brian Havard | "He is not the messiah! |
| brianh@kheldar.apana.org.au | He's a very naughty boy!" - Life of Brian |
------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, 24 Nov 1999, Greg Marr wrote:
> At 03:03 AM 11/24/99 , you wrote:
> >On Tue, 23 Nov 1999, Ryan Bloom wrote:
> > > libraries shouldn't be killing off processes because there is a
> > problem,
> > > they should alert the program that there is a problem. If we
> > aren't
> > > checking the return code, we should be. That will be fixed in
> > the next
> > > few patches.
> >
> >That's a cop-out. We are talking Apache here.
>
> No, we are talking APR here. APR is not Apache.
By "talking Apache here", I meant "Apache-using-APR". In that condition,
everything ever written for Apache assumes that a memory allocation will
SUCCEED. So, in light of Apache... yes, we should be killing the process
if we can't get the memory.
I also maintain that we must at least attempt to log the error somewhere,
rather than silently die.
> >All throughout Apache, we allocate memory with the presumption that
> >if the function returns, then we *have* allocated that memory.
> >
> >You simply CANNOT change those semantics right now.
>
> Hey, if he wants to change the semantics, which involves adding
> return value checks to all the calls, then I say go ahead. If you
> REALLY need to have a function you can call from within Apache that
> aborts if memory isn't available, then add a wrapper function *in
> Apache* that calls APR and aborts if memory isn't available.
And I say no. The current model of "alloc and you'll get it" means that
Apache can be very fast. It doesn't have to worry about not getting
memory.
If you *do* have to worry about it, then you start putting checks on every
darn function call. You ever see what that looks like? Go look at some COM
code in Windows. You have one line of work, three lines of error handling.
It is absolutely horrible. Further, the time to check a result, when it is
typically successful is just wasted time. Lastly, people will just start
getting lazy and not putting in checks. Then you end up with a case where
a NULL pointer gets hit at some arbitrary point in the code, a long ways
away from the (failed) allocation. Tracking that back is a bitch.
Changing return codes is one thing. Changing the basic programming model
and semantics of Apache development is something else entirely.
I could be mistaken on Ryan's intent (and I asked him that in my last
email), but if we are really talking about needing to check a status code
from ap_pstrup(), then I'll be quite upset.
If Apache is going to provide abort-on-malloc-failure wrappers, then that
is somewhat acceptable. But there would be a lot of wrappers, and a lot of
time cycling through those things.
> >Within APR (or Apache itself for that matter), you can then just
> >call something like
> >ap_abort_with_msg("whatever"). It will ensure the message is written
> >to the platform-specific place and abort().
> >
> >This is APR... we can use platform-specific mechanisms to log these
> >messages.
>
> If APR does have this function, it should NEVER call it from within
> APR. Better yet, have ap_log_message and then have Apache call
> ap_log_message() and abort() or exit().
Apache has its own logging mechanisms that it can use at its level. I'm
talking about a "last resort" logging mechanism that APR can use.
> >Really: we can't just abort() without telling the user what
> >happened. I've been bitten by that several times in the past and it
> >was a pain in the ass. And hell... I'm a programmer with debug tools
> >available! I hate to think what would happen to Joe Sysadmin.
>
> APR is a library, it can't abort at all. Apache, on the other can,
> can abort. Then again, should a single failed malloc be enough to
> bring down the entire system without cleaning anything up?
Sure, APR is a library, but it is still an integral part of Apache and one
of its goals is to be well-tuned for Apache's needs. And yes, I think a
malloc should bring things down. It is a better model than boatloads of
memory-status checks.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c
apr_pools.c apr_tables.c
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 03:03 AM 11/24/99 , you wrote:
>On Tue, 23 Nov 1999, Ryan Bloom wrote:
> > libraries shouldn't be killing off processes because there is a
> problem,
> > they should alert the program that there is a problem. If we
> aren't
> > checking the return code, we should be. That will be fixed in
> the next
> > few patches.
>
>That's a cop-out. We are talking Apache here.
No, we are talking APR here. APR is not Apache.
>All throughout Apache, we allocate memory with the presumption that
>if the function returns, then we *have* allocated that memory.
>
>You simply CANNOT change those semantics right now.
Hey, if he wants to change the semantics, which involves adding
return value checks to all the calls, then I say go ahead. If you
REALLY need to have a function you can call from within Apache that
aborts if memory isn't available, then add a wrapper function *in
Apache* that calls APR and aborts if memory isn't available.
>Within APR (or Apache itself for that matter), you can then just
>call something like
>ap_abort_with_msg("whatever"). It will ensure the message is written
>to the platform-specific place and abort().
>
>This is APR... we can use platform-specific mechanisms to log these
>messages.
If APR does have this function, it should NEVER call it from within
APR. Better yet, have ap_log_message and then have Apache call
ap_log_message() and abort() or exit().
>Really: we can't just abort() without telling the user what
>happened. I've been bitten by that several times in the past and it
>was a pain in the ass. And hell... I'm a programmer with debug tools
>available! I hate to think what would happen to Joe Sysadmin.
APR is a library, it can't abort at all. Apache, on the other can,
can abort. Then again, should a single failed malloc be enough to
bring down the entire system without cleaning anything up?
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > which is the end goal, "Make APR useful for other applications". Give me
> > some time to finish everything.
>
> Are you actually stating that every time we call ap_pstrdup(), we now have
> to check its result?
Yep. The same way everytime you call strdup, you should be checking a
return code. There is a reason libraries don't just abort. It isn't
their job. If you program wants to abort bceause it can't get memory,
then you can abort, APR can't make that decision for you.
> > And on Mac OS, or BEOS? Your method only works if we already know which
> > platform we are running on when we write the code, which pretty much means
> > ./configure ; make doesn't live up to it's full potential. This method
>
> What?
>
> Write one chunk for Windows, one chunk for Unix, one for Mac. That's what
> APR is about, I thought? Allow you to express a semantic in a portable
> fashion.
>
> I think we're miscommunicating here.
I understand what you are getting at perfectly. It just doesn't work. It
did work in Apache, because we always mapped stderr to the error_log, so
when we wrote to stderr, it always went to the error_log. On any platform
we don't specifically code for, the error message could get lost. Apache
knows where the error log is supposed to go, NOT APR.
Here's another issue. Let's assume my box is running two copies of
Apache on WinNT (a common way to run a proxy server), and APR aborts and
puts a message into the System Log, which Apache had the problem? There
is no way for APR to tell you which Apache died. By letting Apache write
the error itself, the error message can ALWAYS go to a useful place, and
it is where Apache users expect those messages to go.
> But they GET ONE. We're talking a totally abnormal condition, where we
> just can't get a log of the error to the right place.
>
> On Unix, that goes to stderr. That is quite standard. On Windows, that
> goes to the Event Log. Again: standard.
They'll still get one, it will just be Apache writing it in a portable way
to the error log. Instead of APR die'ing on them and writing a message
somewhere on the system.
> I realize that you're still working on this. But it sounds (and looks)
> like the direction you're going will require us to check every
> ap_psprintf() result, every ap_pstrcat(), every ...
>
> -1 if that's the case.
Yep, we're going to have to check return codes. That's life. Here's one
more argument for you. I don't want to put too much emphasis on this,
because right now APR is only used for Apache. What happens when APR is
used by another application, which doesn't have the multi-process model of
Apache? Does APR just quit without ever notifying the application? It
isn't up to a library to say sorry, I'm taking us down now, it's up to the
application to do that.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Wed, 24 Nov 1999, Ryan Bloom wrote:
> How is this a cop-out? I am going to be going through and checking return
> codes. I am working on this. I said that in the last message. I am not
> going to leave things in the current state. If I can't change this now,
> it can NEVER change. Apache 2.0 is the best place to change the behavior
> of some API's, that's what I'm doing. Having a process just die because a
> library decided it couldn't do anything is the cop-out. Having the
> library return gracefully with an error condition is the "right way" to
> handle these cases, and it makes sense for applications other than Apache,
> which is the end goal, "Make APR useful for other applications". Give me
> some time to finish everything.
Are you actually stating that every time we call ap_pstrdup(), we now have
to check its result?
>...
> And on Mac OS, or BEOS? Your method only works if we already know which
> platform we are running on when we write the code, which pretty much means
> ./configure ; make doesn't live up to it's full potential. This method
What?
Write one chunk for Windows, one chunk for Unix, one for Mac. That's what
APR is about, I thought? Allow you to express a semantic in a portable
fashion.
I think we're miscommunicating here.
I'm suggesting replacing fprintf(stderr, ...) with
ap_log_of_last_resort(...). The function is implemented in each of APR's
per-platform directories.
> also puts Apache error logs all over the place. Windows doesn't map the
> error log to the Apache log files. So, on any non-unix system, where APR
> decideds to just abort, the message for why we aborted isn't where people
> expect it.
But they GET ONE. We're talking a totally abnormal condition, where we
just can't get a log of the error to the right place.
On Unix, that goes to stderr. That is quite standard. On Windows, that
goes to the Event Log. Again: standard.
>...
> Please read the message I sent last time again. We AREN'T GOING to just
> abort without a message. APR isn't going to abort at all anymore. I'm
> still finishing this peice of work.
I realize that you're still working on this. But it sounds (and looks)
like the direction you're going will require us to check every
ap_psprintf() result, every ap_pstrcat(), every ...
-1 if that's the case.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > libraries shouldn't be killing off processes because there is a problem,
> > they should alert the program that there is a problem. If we aren't
> > checking the return code, we should be. That will be fixed in the next
> > few patches.
>
> That's a cop-out. We are talking Apache here. All throughout Apache, we
> allocate memory with the presumption that if the function returns, then we
> *have* allocated that memory.
>
> You simply CANNOT change those semantics right now.
>
> In other words: we *are* going to abort(). And it is very incorrect to
> abort without even attempting to state what happened.
>
> [ caveat: if you want to check stuff *withing* APR, then okay. but APIs
> exposed to Apache *never* return an "out of memory" error. ]
How is this a cop-out? I am going to be going through and checking return
codes. I am working on this. I said that in the last message. I am not
going to leave things in the current state. If I can't change this now,
it can NEVER change. Apache 2.0 is the best place to change the behavior
of some API's, that's what I'm doing. Having a process just die because a
library decided it couldn't do anything is the cop-out. Having the
library return gracefully with an error condition is the "right way" to
handle these cases, and it makes sense for applications other than Apache,
which is the end goal, "Make APR useful for other applications". Give me
some time to finish everything.
> > There is no good way to write an error message in a platform independant
> > way. I am removing the exit calls over time, but the fprintf's go away
> > now, because they don't work on all systems.
>
> Yes there is. There is a very good way, and I outlined it previously.
> I'll state it one more time:
>
> On Windows: put the message into the Event Log.
> On Unix: write it to stderr.
And on Mac OS, or BEOS? Your method only works if we already know which
platform we are running on when we write the code, which pretty much means
./configure ; make doesn't live up to it's full potential. This method
also puts Apache error logs all over the place. Windows doesn't map the
error log to the Apache log files. So, on any non-unix system, where APR
decideds to just abort, the message for why we aborted isn't where people
expect it.
>
> In fact, these are the "right ways" to do this for each platform. We can
> use it. If we run into that stupid NT vs Win9x in that no event log
> exists, then write it to a damn text file in "C:\ApacheErrors.txt". When
> APR is loaded, then open the Event Log or output file (so you won't have
> to allocate any resources when it comes time to use them). Within APR (or
> Apache itself for that matter), you can then just call something like
> ap_abort_with_msg("whatever"). It will ensure the message is written to
> the platform-specific place and abort().
>
> This is APR... we can use platform-specific mechanisms to log these
> messages.
>
> Really: we can't just abort() without telling the user what happened. I've
> been bitten by that several times in the past and it was a pain in the
> ass. And hell... I'm a programmer with debug tools available! I hate to
> think what would happen to Joe Sysadmin.
Please read the message I sent last time again. We AREN'T GOING to just
abort without a message. APR isn't going to abort at all anymore. I'm
still finishing this peice of work.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Greg Stein <gs...@lyra.org>.
On Tue, 23 Nov 1999, Ryan Bloom wrote:
> On Tue, 23 Nov 1999, Greg Stein wrote:
> > On 23 Nov 1999 rbb@hyperreal.org wrote:
> > >...
> > > --- apr_execve.c 1999/11/22 18:01:14 1.4
> > > +++ apr_execve.c 1999/11/23 21:22:45 1.5
> > > @@ -349,7 +349,6 @@
> > > newargv = (char **) malloc((p - lbuf + 1)
> > > + (i + sargc + 1) * sizeof(*newargv));
> > > if (newargv == NULL) {
> > > - fprintf(stderr, "Ouch! Out of memory in hashbang()!\n");
> > > return NULL;
> >
> > Do callers check this return code? Should we have an abort() here?
>
> libraries shouldn't be killing off processes because there is a problem,
> they should alert the program that there is a problem. If we aren't
> checking the return code, we should be. That will be fixed in the next
> few patches.
That's a cop-out. We are talking Apache here. All throughout Apache, we
allocate memory with the presumption that if the function returns, then we
*have* allocated that memory.
You simply CANNOT change those semantics right now.
In other words: we *are* going to abort(). And it is very incorrect to
abort without even attempting to state what happened.
[ caveat: if you want to check stuff *withing* APR, then okay. but APIs
exposed to Apache *never* return an "out of memory" error. ]
>...
> > I still think we need a platform-dependent way to log this kind of error
> > message. Apache should NOT simply exit without saying anything. The admin
> > could be scratching their head for weeks until they figure out "oh! maybe
> > it is running out of memory!"
> >
> > -0 on abort() with no message.
>
> There is no good way to write an error message in a platform independant
> way. I am removing the exit calls over time, but the fprintf's go away
> now, because they don't work on all systems.
Yes there is. There is a very good way, and I outlined it previously.
I'll state it one more time:
On Windows: put the message into the Event Log.
On Unix: write it to stderr.
In fact, these are the "right ways" to do this for each platform. We can
use it. If we run into that stupid NT vs Win9x in that no event log
exists, then write it to a damn text file in "C:\ApacheErrors.txt". When
APR is loaded, then open the Event Log or output file (so you won't have
to allocate any resources when it comes time to use them). Within APR (or
Apache itself for that matter), you can then just call something like
ap_abort_with_msg("whatever"). It will ensure the message is written to
the platform-specific place and abort().
This is APR... we can use platform-specific mechanisms to log these
messages.
Really: we can't just abort() without telling the user what happened. I've
been bitten by that several times in the past and it was a pain in the
ass. And hell... I'm a programmer with debug tools available! I hate to
think what would happen to Joe Sysadmin.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c
apr_tables.c
Posted by Ryan Bloom <rb...@ntrnet.net>.
On Tue, 23 Nov 1999, Greg Stein wrote:
> On 23 Nov 1999 rbb@hyperreal.org wrote:
> >...
> > --- apr_execve.c 1999/11/22 18:01:14 1.4
> > +++ apr_execve.c 1999/11/23 21:22:45 1.5
> > @@ -349,7 +349,6 @@
> > newargv = (char **) malloc((p - lbuf + 1)
> > + (i + sargc + 1) * sizeof(*newargv));
> > if (newargv == NULL) {
> > - fprintf(stderr, "Ouch! Out of memory in hashbang()!\n");
> > return NULL;
>
> Do callers check this return code? Should we have an abort() here?
libraries shouldn't be killing off processes because there is a problem,
they should alert the program that there is a problem. If we aren't
checking the return code, we should be. That will be fixed in the next
few patches.
> > blok = (union block_hdr *) malloc(size + sizeof(union block_hdr));
> > if (blok == NULL) {
> > - fprintf(stderr, "Ouch! malloc failed in malloc_block()\n");
> > - exit(1);
> > + return NULL;
>
> This doesn't look good! A caller might get a NULL pointer now, whereas
> before they were guaranteed not to (cuz the program would dump rather than
> doing so).
>
> >...
> > @@ -299,7 +298,6 @@
> > "at the end of a block!\n");
> > while (free_blk) {
> > if (free_blk == blok) {
> > - fprintf(stderr, "Ouch! Freeing free block\n");
> > abort();
> > exit(1);
>
> I still think we need a platform-dependent way to log this kind of error
> message. Apache should NOT simply exit without saying anything. The admin
> could be scratching their head for weeks until they figure out "oh! maybe
> it is running out of memory!"
>
> -0 on abort() with no message.
There is no good way to write an error message in a platform independant
way. I am removing the exit calls over time, but the fprintf's go away
now, because they don't work on all systems.
>
> (this applies to the other places where you removed a message before an
> abort())
>
> >...
> > --- apr_tables.c 1999/10/17 16:11:43 1.5
> > +++ apr_tables.c 1999/11/23 21:22:48 1.6
> > @@ -284,9 +284,7 @@
> > static ap_table_entry_t *table_push(ap_table_t *t)
> > {
> > if (t->a.nelts == t->a.nalloc) {
> > - fprintf(stderr,
> > - "table_push: ap_table_t created by %p hit limit of %u\n",
> > - t->creator, t->a.nalloc);
> > + return NULL;
> > }
> > return (ap_table_entry_t *) ap_push_array(&t->a);
>
> In the old behavior, the caller would always get memory back. This is no
> longer the case. We should either abort (with a message!) or go ahead and
> log the problem, but return the memory anyhow.
My bad, this one I'll fix tomorrow.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@ntrnet.net
6209 H Shanda Dr.
Raleigh, NC 27609 Ryan Bloom -- thinker, adventurer, artist,
writer, but mostly, friend.
-------------------------------------------------------------------------------