Subject: Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests

From: Alexander Klauer <Alexander.Klauer_at_itwm.fraunhofer.de>
Date: Thu, 21 Mar 2013 14:37:18 +0100

On 03/21/2013 01:44 PM, Tommie Gannert wrote:
> 2013/3/21 Alexander Klauer <Alexander.Klauer_at_itwm.fraunhofer.de>:
>> How about this:
>>
>> * Make ares_destroy() more user friendly and allow callbacks to register new
>> requests on the channel being destroyed. Those requests would then
>> immediately be finished with ARES_EDESTRUCTION.
>> * Let ares_cancel() walk through the list, but don't assert it's empty
>> afterwards.
>
> +1 on both accounts, but I guess that doesn't fully solve your issue. As a user
> of the library, I would think that semantic is sound. Possibly with the addition
> that ares_cancel() should not (accidentally) cancel any new requests created
> within it, if that could happen.

Sure.

What happens if someone calls ares_cancel() while ares_cancel() is in
progress? With (1), that would mean that the topmost (most recent)
instance ares_cancel() has to cancel all those requests that appeared
since the next-to-topmost instance was invoked, right?

>> Then, to get behaviour (1), one would call ares_cancel(), and for behaviour
>> (2), you would use ares_destroy(), followed by ares_init(). A little awkward
>> perhaps, but the API would not be broken.
>>
>> Thoughts?
>
> I'm curious what your use case is, and I agree it's awkward to have to use
> ares_destroy() if you don't really mean to destroy it.

No real use case, my leaning towards (2) is just personal feeling. My
actual use case right now is to call ares_cancel() immediately prior to
ares_destroy(). With the implementation of (1), I would of course simply
call ares_destroy(). I also call ares_cancel() in an out of memory
condition.

Come to think of it, ares_cancel() is pretty useless. Normally, you
would either want to cancel one single request, the functionality for
which is missing, or cancel everything when you destroy the channel.

> *Thinking out loud* Could one perhaps add a channel flag that would alter
> the behaviour of ares_cancel()? Basically a boolean disabling the whole
> library. Does that make sense? I don't know of any other library doing that,
> except for "shutdown," which is what we/you would like to avoid.

I think you mean the whole channel, not the whole library. I'm not sure
that's a good idea. All the other public flags are about what ares does
with stuff sent and received over the network, not internal behaviour.
shutdown() has a specific use in the case of TCP connections (and it
does get its own flags). OTOH, my current implementation of (2) uses
just that flag variable internally.

Best regards,
Alexander
Received on 2013-03-21