Re: cvs commit: apache-2.0/src/lib/apr/lib apr_execve.c apr_pools.c apr_tables.c

[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

[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/