Re: [RFC PATCH] Re: PAC serving

On Wed, 14 Jul 2010 15:34:18 -0600, Alex Rousskov
<rousskov_at_measurement-factory.com> 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:
>
> * 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:

Sweet. Simpler code :)

>
> - Call mustStop(reason) for errors that require further processing in
> the same method(s) chain, below/after the mustStop() call. Efficient.
>

Thanks. Some of the file_fd error handling will be using that.

> - 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.
>

Aye. Noted.

>
> * 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.
>

Sigh. Okay.

>
> * 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.
>

Ah.

>
> * 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.
>
>
> * 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...
>
>
> * If a job does not have a doneAll() method implemented, it is probably
> buggy. Any job must know what it wants to accomplish. Please note that
> doneAll() is for defining the successful termination goal/condition.
> Errors are handled by mustStop() or throw, as discussed above.
>
> Similarly, if your doneAll() implementation looks like "return isDone;",
> you are doing it wrong. Compute the condition directly rather than
> expecting various job methods to maintain some isDone variable
correctly.
>

Okay. That answers a Q or two I had on done(0 states. Cheers.

>
> * Ask yourself what the user will see/experience when the job throws,
> which could happen as early as in the start() method (technically, it
> can happen even earlier, in the job creation or creator code). Are you
> OK with that?
>

Um. the common ones yes. Still thinking of side cases.

Amos
Received on Thu Jul 15 2010 - 00:07:54 MDT