Re: [RFC PATCH] Re: PAC serving

Alex Rousskov wrote:
> On 07/14/2010 08:02 AM, Amos Jeffries wrote:
>> Since no description beats the real code here is the current patch I have.
>>
>> If you guys can't find any other major issues with this I'll go ahead
>> and make it build and test it out.
>
> I have some comments, but they should not stop you from running and
> testing the code, of course:

Taking these into account on the cleanup-comm usage of AsyncJob has got
me around a lot of weird problems.
  I think this list or a very similar should be added to AsyncJob.h
class documentation. What is there now leaves people like myself using
the API wrong.

>
> * Never call swanSong() directly. If you are outside an AsyncCall
> handler, and want to kill the job, then call deleteThis(). If you are
> inside an AsyncCall handler, you have several options for job termination:
>
> - Call mustStop(reason) for errors that require further processing in
> the same method(s) chain, below/after the mustStop() call. Efficient.
>
> - Throw (via Must or directly) for errors that do not require further
> processing in the same method(s) chain, below/after the mustStop() call.
> Inefficient but simple and allows exiting from deeply nested method calls.
>
> - Otherwise, just finish the call. Your doneAll() should return true
> and the job will terminate successfully.
>
> swanSong() will be called automatically in all of these cases when the
> job is being terminated. It is a general cleanup method, like a
> destructor. The only difference is that a destructor must not throw.
>
>
> * Do not assume swanSong() is called in some perfectly nice job state.
> The job code or the code it calls may throw at any time after start()
> was called. The entry may be gone, the Abort may have been called, the
> fd may have been closed, etc.
>
>
> * Never call deleteThis() in contexts other than those documented above.
> It is a hack for the old-style code. You can avoid it and other
> old-style special precautions altogether if you convert sync calls into
> async ones. This is especially easy for old-style calls that have only
> one parameter ("data") or two simple parameters.
>
>
> * In swanSong, always call swanSong() of the parent, after you are done
> cleaning up your job. It does not matter whether the [current] parent
> swanSong() does nothing.
>
>
> * If a job does Comm I/O, it probably needs a Comm closing handler.
>
>
> * To start a job, use AsyncStart. Do not call start() directly. There
> are examples in the code.

Is that true in ALL cases both sync and async?

In this particular PAC case, we want the code of start() to be run first
as sync to grab small files as fast as possible only ending with a
folowup async loop if the first does not complete in time.
  The first and subsequent actual Call is scheduled at the end of start.

>
> * Starting a job from a Comm handler is fine, but that handler should
> not be a part of the job you are starting. Start the job in the caller
> and let the job register its needs with Comm...

This still general or do you see a particular violation I've done?
  Noting that I call start() sync as above and let the start() routine
do all the registering bits.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.5