Re: [PATCH] Delete server-side deleteThis()

On 10/13/2011 07:50 AM, Tsantilas Christos wrote:
> On 10/13/2011 02:55 PM, Amos Jeffries wrote:
>> On 13/10/11 22:02, Tsantilas Christos wrote:
>>> This patch finishes the conversion of ServerStateData into AsyncJob by
>>> properly implementing the doneAll() method and by removing calls to
>>> deleteThis() or replacing with mustStop() calls as appropriate.

>> In the new doneAll() you seem to have dropped:
>> -#if USE_ADAPTATION
>> - Adaptation::Initiator::doneAll() &&
>> - BodyProducer::doneAll() &&
>> -#endif
>> - BodyConsumer::doneAll()
>>
>> ... IIRC that goes against the AsynJob API requirements. If it is
>> supposed to be dropped please add a comment about why.

> None of the above "doneAll()" methods is really implemented. The default
> AsynJob::doneAll() used for all of them. The default doneAll() returns
> always true.

Christos,

    While you are correct, I do agree with Amos. We should add those
calls back even though they all _currently_ return true. Checking with
parents is a part of the doneAll() API. You can place those always-true
calls last to minimize their tiny performance impact, if any.

Sorry I missed that myself.

Thank you,

Alex.
Received on Thu Oct 13 2011 - 14:39:51 MDT