Andrew, Here is a link to the tiny bit of code which handles every request:
http://junom.com/gitweb/gitweb.perl?p=aolserver.git;a=blob;f=aolserver/nsd/queue.c;hb=HEAD#l585 >From my reading of this, the problem must be that ns_adp_abort is not returning NS_OK or NS_FILTER_RETURN. It probably should return NS_OK once the entire adp stack is unwound (this will happen in the C code somewhere, but I haven't ever looked at how adp processing works). Otherwise on line 625 of nsd/queue.c, the trace filters are skipped. Looking at this code it appears that preauth, postauth and request proc errors are handled differently. Only errors thrown in a preauth filter are nullified, which allows trace filters to run. This seems strange. tom jackson On Thu, 2009-04-02 at 18:03 -0500, Andrew Steets wrote: > What was the original purpose of "trace" filters? At the C API level > there is a distinction between between a trace filter and a cleanup > callback, but it doesn't look like you can register a cleanup proc > from TCL. Maybe this was mistakenly omitted? > > The cleanup procs run unconditionally. It seems like that is the most > appropriate place to handle "cleanup of resources." Alternatively we > could change the trace filters to run regardless of the > Ns_ConnRunRequest() return status, but then that would make them > basically the same as the cleanups. > > I looked a little deeper into the source. The confusion seems to > arise in NsAdpFlush() which is run at the end of all ADP processing. > The code there is smart enough to recognize when an abort exception > has been signalled; it sets the TCL result to "adp flush disabled: adp > aborted", but it still returns TCL_ERROR. That is essentially where > the TCL exception gets turned into a full blown connection processing > error. We could change NsAdpFlush() to return success when it > recognizes the abort exception, or just not run NsAdpFlush() for abort > exceptions. > > There would still be cases where trace filters would not run though. > For instance if you called ns_returnxxx without calling ns_adp_abort. > I'm not sure if that is a bad thing. > > It would be nice to hear from anyone who knows about the original > motivation for the trace and cleanup filters. > > -Andrew > > On Thu, Apr 2, 2009 at 3:53 PM, Tom Jackson <[email protected]> wrote: > Gustaf, > > You may be "using" traces but not realize it, it sounds like > ns_adp_abort isn't don't what was originally intended. > > I wouldn't worry about an runtime error caused during running > traces, it > would be an error to even use ns_adp_abort in a trace filter > because the > connection is already finished. This is analogous to calling > [break] > outside of a loop. > > It seems important to consider ns_adp_abort, ns_adp_return and > ns_adp_break as a unit. They add necessary loop type controls > so that > developers can create deeply nested code and still get out of > it without > the need to use [catch]. But, like a lot of AOLserver specific > procedures, there is no hand-holding in their use. They can be > misued. > > In this particular case, it looks like somewhere along the > way, > ns_adp_abort was modified to not work as expected. > > The desired effect is exactly what you would get by returning > filter_return from a preauth or postauth filter. This effect > is to skip > to trace filters, not past them. > > Skipping trace filters even on an aborted connection would be > a disaster > for any application which relies on cleanup of resources. > > tom jackson > > > On Thu, 2009-04-02 at 11:12 +0200, Gustaf Neumann wrote: > > Andrew Steets schrieb: > > > The patch I sent earlier seems to fulfill these needs, but > I am > > > worried about corner cases where LogTrace (from the nslog > module) > > > could blow up. Nothing about the state of the Conn * > seems to be > > > guaranteed when the ConnCleanup callbacks are called. > > > > > Dear Andrew, > > > > i think most (all?) of the repondents seems to agree that > writing in the > > about case to > > the access log file. For me there are still two quesions > open: > > > > a) is it possoble to call ns_adp_abort at some time, where > the server > > might crash > > (in normal operations, everthing looks fine to me, > problems might > > occur in > > when called from some traces; other calls are likely to > have similar > > problems) > > > > b) the patch replaces the call to the regular server trace > by a > > connection cleanup call. > > this means, at least in 4.5.*, ns_adp_abort seems to > cancel all > > traces (also > > these registered with ns_register_trace). Is this > desired? > > > > From Tom's website: > http://rmadilo.com/files/nsapi/ns_adp_abort.html > > the doc of ns_adp_abort says > > > > ... Every ns_returnxxx call in an ADP should be followed > with a call > > to ns_adp_abort.... > > > > With this recommendation, cancelling traces seem wrong to > me; or at > > least, > > this should be documented. > > > > We don't use traces, all of OpenACS does not use it, so this > is no > > current issue for us. > > > > -gustaf neumann > > > > > > -- > > AOLserver - http://www.aolserver.com/ > > > > To Remove yourself from this list, simply send an email to > <[email protected]> with the > > body of "SIGNOFF AOLSERVER" in the email message. You can > leave the Subject: field of your email blank. > > > > > -- > AOLserver - http://www.aolserver.com/ > > To Remove yourself from this list, simply send an email to > <[email protected]> with the > body of "SIGNOFF AOLSERVER" in the email message. You can > leave the Subject: field of your email blank. > > > > -- > AOLserver - http://www.aolserver.com/ > > > > To Remove yourself from this list, simply send an email to > <[email protected]> with the > body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: > field of your email blank. > > -- AOLserver - http://www.aolserver.com/ To Remove yourself from this list, simply send an email to <[email protected]> with the body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: field of your email blank.
