httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: logging APR errors
Date Mon, 17 Jul 2000 19:54:57 GMT

> > This design doesn't solve this problem, in fact, IMNSHO it makes it
> > worse.  Think of it this way.  What if Apache doesn't want to log on a
> > given error.  Say for ap_sendfile, there is at least one condition we
> > don't want to log, but it requires the ability to make the call-back
> > (otherwise we are Apache centric).  Apache must de-register the callback
> > before calling ap_sendfile.  And then re-register it after calling
> > ap_sendfile, or Apache will need to register/de-register before EVERY APR
> > call.  There is only one way aroung this, which is to only make the
> > callback in the cases Apache cares about, which ties APR to Apache, which
> > is a BIG no-no.
> 
> I seriously doubt that Apache or any other app is going to want to do
> such a thing, but obviously it is a possibility.

Take a closer look.  I didn't even have to really search the code.  At
times, Apache logs errors for ap_stat, at other times it doesn't.  I would
bet there are more functions like that.

> > Too bad.  You can't add code in some cases and not others.  That ties you
> > to Apache.
> 
> I don't see how this is Apache-centric.  It is just common sense to
> me.  Consider a call to ap_listen() which fails.  FOR ANY APP, what
> would this APR function pass to an error call-back to help debug the
> problem?  There is only one syscall there and there is no mystery
> about the parameters.  This is not the type of problem that we need to
> solve.  It is already solved in fact since the caller of this
> functions can log the error, as the caller has all possible info.

So, what you are advocating, is magically logging the error in some cases,
and making the application log the error in other cases.  This is
ugly.  This will look like:

ap_stat(...)
ap_log_error(.... "Stat failed");

ap_sendfile(...)
return;

This doesn't seem like an awful hack to you?  You are logging some errors
in your app, and some errors are just magically logged to the log file.

> > Yes, this is true, iff we only make the callback in some places, we don't
> > add extra function calls to the non-error cases.  Since we can't just make
> > the callback in some places, we have to add at least one extra call in ALL
> > cases.
> 
> I don't understand any of your response.  There is never call-back
> logic except in an error path.  Whether or not we code the call-back
> logic in a certain error path (e.g., when shutdown() fails) has no
> bearing on any other place we might think about coding the call-back
> logic. 

The problem comes, with a function that we sometimes log failures, and
other times don't.

> I can see why putting a string in the pool is a problem.  I don't see
> why the error call-back is a problem.  This blindness may be due to
> assuming that the error call-back's user data will stay the same for
> the life of a pool.  Can you please explain the thread-safe issue?

How we log is based on where we are in the code, and what information we
have access to.  Look at scan_script_header_err_core in util_script.  In
the same file, we are logging using ap_log_error and ap_log_rerror.  This
example isn't perfect, because we aren't logging based on function return
values, but it will demonstrate the problem, and I found it in under a
minute, so if I had more time, I believe I could find an exact example of
the problem.

If we pass the same pool with the same user_data to two different APR
functions, we will always log using the same error function
(ap_log_error/ap_log_rerror).  Sometimes, even if we have a request_rec,
we don't want to use ap_log_rerror.  This means that in order to convey
that to the logging callback, we would have to modify the user_data in
some way.  Either with a flag, or by removing the request_rec.  Bang, you
hit thread-safety issues as soon as you have to modify the user-data.

> > The design I am leaning most towards right now, is for APR to define a
> > bunch more error values, and just return them in the correct spots.
> 
> I'll look forward to seeing it.

The code is there, and the design is done.  Add a couple of more error
values, and return them where appropriate.  This lets the App deal with
errors, which is the correct solution.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message