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

Reply via email to