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

2013/3/21 Alexander Klauer <Alexander.Klauer_at_itwm.fraunhofer.de>:
> 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?

Good corner case.

I try to think of how I would expect it to work if it was just
multiple threads doing
requests and ares_cancel() would cancel them. And I think what you suggest
makes a lot of sense. The second call would be responsible for cancelling all
active requests though, so if you have three requests, A B C, and cancelling A
causes a request D to be created, and then B calls cancel(), then you would
have a request list like "C D" when entering that second cancel(). In that case
both the first and second cancel() would be responsible for cancelling C, and
it would have to be cancelled before the second cancel() returns (and implicitly
thus also the first as they were recursive calls.)

Might be tricky to get right without ref counting. If it requires a lot of code
changes, maybe it's not worth enforcing such strict semantics, though.

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

Then it sounds like the proposed functionality would actually benefit your code?

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

Yupp, I know. :)

It was one of the biggest surprises when I started using it.

>> *Thinking out loud* Could one perhaps add a channel flag that would alter
>
> I think you mean the whole channel, not the whole library.

Ah, yes.

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

Aight, let's scrap that line of thought.

-- 
Tommie