Re: integration of c-ares into various file descriptor based main loops
On 11/07/2013 01:44 PM, Daniel Stenberg wrote: > On Wed, 6 Nov 2013, Jakub Hrozek wrote: > >> Maybe you could also include libverto. It's relatively new event loop >> library that encapsulates other event loop approaches as back ends. >> It's being used by MIT Kerberos and GSS-Proxy among others. > > I would say that the most sensible thing for a library to do is to > provide callbacks for what file descriptors to wait for (ADD, REMOVE, > MODIFY) activities on, and what those activities are. And a separate one > that sets a timeout timer if necessary. > > That should be possible to make work with any event library or select() > / poll() or whatever out there. > > Not completely out of coincidense, that's the API libcurl provides for > doing event-based operations... I concur, a library should not bind itself to a specififc event handling framework, especially because two different approaches in the same application may be difficult to reconcile with each other. But, since you mentioned timeouts, I start wondering whether it is such a good idea to communicate only one single timeout. Doing so means you have to maintain a priority queue of timeouts somewhere under the hood, which is a complex thing. So, why not simply let the users know the timeout for each fd, so they can put it in some priority queue (such as the one provided by the event loop library) together with all the other stuff that goes in there (one timeout queue per application). This also lets the users make the design choices the library author does not have enough information to make. Best regards, Alexander
Re: [PATCH 2/3] library init: documentation update
Hi, On 03/18/2013 09:26 AM, Alexander Klauer wrote: OK, I've updated the commit. sorry for being a pest, but this patchset still has not made it into the master branch at https://github.com/bagder/c-ares/commits/master. Is there still something wrong with it? What can I do to help? Best regards, Alexander
Re: [PATCH 2/3] library init: documentation update
Hi Günter, On 04/10/2013 12:44 PM, Guenter wrote: Hi Alexander, On 10.04.2013 10:03, Alexander Klauer wrote: Any further comments? If not, could this patchset be committed, please? from what I see your patches are already committed: https://github.com/bagder/c-ares/commits/master on that page, I see only my ares_cancel() patchset, not the ares_library_init()/_cleanup() one. Best regards, Alexander
Re: [PATCH 2/3] library init: documentation update
On 04/10/2013 11:07 AM, Alexander Klauer wrote: Hi, no need to CC me, I'm subscribed. On 04/10/2013 10:44 AM, Saúl Ibarra Corretgé wrote: On 4/10/13 10:03 AM, Alexander Klauer wrote: On 03/18/2013 09:26 AM, Alexander Klauer wrote: On 03/15/2013 08:26 PM, Yang Tse wrote: On Fri, Mar 15, 2013, Alexander Klauer wrote: This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the new recursive behaviour. It might be better to use "Its reference counted initialize/deinitialize behavior," or something similar, instead of "Its recursive behavior," OK, I've updated the commit. Any further comments? If not, could this patchset be committed, please? TIA. I'm no expert, but I thought I'd throw my 2 cents here :-) What about using something like pthread_once? Would that work for you? Not sure if that's supported in all platforms though. That would work on systems where it's supported, but unfortunately, it's not portable. Since one of the points of having an asynchronous resolver is not having to deal with threads in the first place, I'd be loath to import a dependency on a threading library into c-ares. I just forgot: pthread_once() would work for the library init, but the finalisation has to be taken care of as well, so you need some kind of reference counting. Best regards, Alexander
Re: [PATCH 2/3] library init: documentation update
Hi, no need to CC me, I'm subscribed. On 04/10/2013 10:44 AM, Saúl Ibarra Corretgé wrote: On 4/10/13 10:03 AM, Alexander Klauer wrote: On 03/18/2013 09:26 AM, Alexander Klauer wrote: On 03/15/2013 08:26 PM, Yang Tse wrote: On Fri, Mar 15, 2013, Alexander Klauer wrote: This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the new recursive behaviour. It might be better to use "Its reference counted initialize/deinitialize behavior," or something similar, instead of "Its recursive behavior," OK, I've updated the commit. Any further comments? If not, could this patchset be committed, please? TIA. I'm no expert, but I thought I'd throw my 2 cents here :-) What about using something like pthread_once? Would that work for you? Not sure if that's supported in all platforms though. That would work on systems where it's supported, but unfortunately, it's not portable. Since one of the points of having an asynchronous resolver is not having to deal with threads in the first place, I'd be loath to import a dependency on a threading library into c-ares. Best regards, Alexander
Re: [PATCH 2/3] library init: documentation update
On 03/18/2013 09:26 AM, Alexander Klauer wrote: On 03/15/2013 08:26 PM, Yang Tse wrote: On Fri, Mar 15, 2013, Alexander Klauer wrote: This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the new recursive behaviour. It might be better to use "Its reference counted initialize/deinitialize behavior," or something similar, instead of "Its recursive behavior," OK, I've updated the commit. Any further comments? If not, could this patchset be committed, please? TIA. Best regards, Alexander
[PATCH 1/4] ares_cancel(): cancel requests safely
An invocation of ares_cancel() walks through the request list, calling the callbacks of all pending requests on a channel. Previously, if such a callback added a new request to the channel, the request list might not end up empty, causing an abort by assertion failure. The present commit ensures that precisely all requests present upon entry of ares_cancel() are cancelled, and that adding new requests through callbacks is safe. --- ares_cancel.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/ares_cancel.c b/ares_cancel.c index e5bb050..465cc9e 100644 --- a/ares_cancel.c +++ b/ares_cancel.c @@ -26,33 +26,33 @@ void ares_cancel(ares_channel channel) { struct query *query; + struct list_node list_head_copy; struct list_node* list_head; struct list_node* list_node; int i; - list_head = &(channel->all_queries); - for (list_node = list_head->next; list_node != list_head; ) + if (!ares__is_list_empty(&(channel->all_queries))) { -query = list_node->data; -list_node = list_node->next; /* since we're deleting the query */ -query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0); -ares__free_query(query); - } -#ifndef NDEBUG - /* Freeing the query should remove it from all the lists in which it sits, - * so all query lists should be empty now. - */ - assert(ares__is_list_empty(&(channel->all_queries))); - for (i = 0; i < ARES_QID_TABLE_SIZE; i++) -{ - assert(ares__is_list_empty(&(channel->queries_by_qid[i]))); -} - for (i = 0; i < ARES_TIMEOUT_TABLE_SIZE; i++) +/* Swap list heads, so that only those queries which were present on entry + * into this function are cancelled. New queries added by callbacks of + * queries being cancelled will not be cancelled themselves. + */ +list_head = &(channel->all_queries); +list_head_copy.prev = list_head->prev; +list_head_copy.next = list_head->next; +list_head_copy.prev->next = &list_head_copy; +list_head_copy.next->prev = &list_head_copy; +list_head->prev = list_head; +list_head->next = list_head; +for (list_node = list_head_copy.next; list_node != &list_head_copy; ) { - assert(ares__is_list_empty(&(channel->queries_by_timeout[i]))); + query = list_node->data; + list_node = list_node->next; /* since we're deleting the query */ + query->callback(query->arg, ARES_ECANCELLED, 0, NULL, 0); + ares__free_query(query); } -#endif - if (!(channel->flags & ARES_FLAG_STAYOPEN)) + } + if (!(channel->flags & ARES_FLAG_STAYOPEN) && ares__is_list_empty(&(channel->all_queries))) { if (channel->servers) { -- 1.7.9.5
[PATCH 4/4] .gitignore: ignore patch files
This commit adds a line to .gitignore to the effect that patch files generated by 'git format-patch' are excluded from the repository. --- .gitignore |1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 19a0eae..0b1087a 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,4 @@ ares_timeout.pdf ares_version.pdf c-ares-*.tar.gz.asc ares_parse_mx_reply.pdf +/[0-9]*.patch -- 1.7.9.5
[PATCH 2/4] Documentation: properly document ARES_ECANCELLED
This commit clarifies the behaviour of ares_cancel() with respect to callbacks and adds missing documentation of ARES_ECANCELLED to the man pages of the affected functions. --- ares_cancel.3|4 +++- ares_gethostbyaddr.3 |3 +++ ares_gethostbyname.3 |3 +++ ares_getnameinfo.3 |3 +++ ares_query.3 |3 +++ ares_search.3|3 +++ ares_send.3 |3 +++ 7 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ares_cancel.3 b/ares_cancel.3 index ef10ec5..0bf6281 100644 --- a/ares_cancel.3 +++ b/ares_cancel.3 @@ -28,7 +28,9 @@ name service channel identified by \fIchannel\fP. \fBares_cancel\fP invokes the callbacks for each pending query on the channel, passing a status of .BR ARES_ECANCELLED . These calls give the callbacks a chance to clean up any state which -might have been stored in their arguments. +might have been stored in their arguments. If such a callback invocation adds +a new request to the channel, that request will \fInot\fP be cancelled by the +current invocation of \fBares_cancel\fP. .SH SEE ALSO .BR ares_init (3) .BR ares_destroy (3) diff --git a/ares_gethostbyaddr.3 b/ares_gethostbyaddr.3 index f589868..7727307 100644 --- a/ares_gethostbyaddr.3 +++ b/ares_gethostbyaddr.3 @@ -70,6 +70,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_gethostbyname.3 b/ares_gethostbyname.3 index a578a53..6b24ea4 100644 --- a/ares_gethostbyname.3 +++ b/ares_gethostbyname.3 @@ -74,6 +74,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_getnameinfo.3 b/ares_getnameinfo.3 index 7e4990c..d227606 100644 --- a/ares_getnameinfo.3 +++ b/ares_getnameinfo.3 @@ -109,6 +109,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_query.3 b/ares_query.3 index 0c5df84..733fbc9 100644 --- a/ares_query.3 +++ b/ares_query.3 @@ -118,6 +118,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_search.3 b/ares_search.3 index 4184b00..2c85d20 100644 --- a/ares_search.3 +++ b/ares_search.3 @@ -119,6 +119,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_send.3 b/ares_send.3 index 48d90ab..b89abfe 100644 --- a/ares_send.3 +++ b/ares_send.3 @@ -73,6 +73,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel -- 1.7.9.5
[PATCH 3/4] ares_destroy() documentation: no new requests
Clarify that no new requests may be added to a resolver channel that is currently being destroyed. --- ares_destroy.3 |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ares_destroy.3 b/ares_destroy.3 index 79171ec..3724df1 100644 --- a/ares_destroy.3 +++ b/ares_destroy.3 @@ -33,7 +33,8 @@ invokes the callbacks for each pending query on the channel, passing a status of .BR ARES_EDESTRUCTION . These calls give the callbacks a chance to clean up any state which -might have been stored in their arguments. +might have been stored in their arguments. A callback must not add new +requests to a channel being destroyed. .SH SEE ALSO .BR ares_init (3), .BR ares_cancel (3) -- 1.7.9.5
Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests
On 03/23/2013 03:12 PM, Daniel Stenberg wrote: On Thu, 21 Mar 2013, Alexander Klauer wrote: Implementation-wise, I think I would just swap the list head in the ares channel with an empty list head on the stack, and then start walking through the list with the list head on the stack. Any new requests would then be added to the now initially empty list in the ares channel. I think this sounds like a really sane approach that is uncomplicated, gives no surprises to a user and is easy to document as well. OK then, I'll implement it this way. Alexander
Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests
On 03/21/2013 02:56 PM, Tommie Gannert wrote: 2013/3/21 Alexander Klauer : 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. Implementation-wise, I think I would just swap the list head in the ares channel with an empty list head on the stack, and then start walking through the list with the list head on the stack. Any new requests would then be added to the now initially empty list in the ares channel. In your example, the first ares_cancel() would cancel A, B, and C, while the second ares_cancel() would cancel only D. This approach also works with more than two ares_cancel()s and no reference counting is needed. 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? It would save one statement, yes. Best regards, Alexander
Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests
On 03/21/2013 01:44 PM, Tommie Gannert wrote: 2013/3/21 Alexander Klauer : 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
Re: [PATCH 1/4] ares_cancel(): ensure cancellation of all requests
On 03/21/2013 11:45 AM, Tommie Gannert wrote: 2013/3/21 Alexander Klauer : An invocation of ares_cancel() walks through the request list, calling the callbacks of all pending requests on a channel. Previously, if such a callback added a new request to the channel, the request list might not end up empty, causing an abort by assertion failure. The present commit ensures that all such newly added requests are cancelled immediately and make it never into request list. Thus, the crash is avoided, and it is made certain that upon return of ares_cancel(), there are no requests whatsoever on the channel. There are two possible contracts I could see for this: 1) ares_cancel() guarantees that all active requests when entering the function will be cancelled. 2) ares_cancel() guarantees there are no active requests when the function exits. The patch clearly implements (2). What are your thoughts on semantics here? I've never had to use ares_cancel(), so I don't know if it currently states (2). The ares_cancel() specification makes no explicit statement regarding callbacks registering new requests. It does use the past tense ("requests made"), but without any indication of whether the point of reference is function entry or function exit, so any case for either possibility will remain weak at best. Personally, I would find it somewhat surprising if I couldn't create new requests within an ares_cancel()-executed callback, and would prefer (1). Funny, for me it's just the other way round. The best solution in such a case would of course be to support both behaviours. One could simply pass some kind of flag to ares_cancel(), but that would break the current API. 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. 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? Best regards, Alexander
[PATCH 3/4] ares_destroy() documentation: no new requests
Clarify that no new requests may be added to a resolver channel that is currently being destroyed. --- ares_destroy.3 |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ares_destroy.3 b/ares_destroy.3 index 79171ec..3724df1 100644 --- a/ares_destroy.3 +++ b/ares_destroy.3 @@ -33,7 +33,8 @@ invokes the callbacks for each pending query on the channel, passing a status of .BR ARES_EDESTRUCTION . These calls give the callbacks a chance to clean up any state which -might have been stored in their arguments. +might have been stored in their arguments. A callback must not add new +requests to a channel being destroyed. .SH SEE ALSO .BR ares_init (3), .BR ares_cancel (3) -- 1.7.9.5
[PATCH 2/4] Documentation: properly document ARES_ECANCELLED
This commit clarifies the behaviour of ares_cancel() with respect to callbacks and adds missing documentation of ARES_ECANCELLED to the man pages of the affected functions. --- ares_cancel.3|4 +++- ares_gethostbyaddr.3 |3 +++ ares_gethostbyname.3 |3 +++ ares_getnameinfo.3 |3 +++ ares_query.3 |3 +++ ares_search.3|3 +++ ares_send.3 |3 +++ 7 files changed, 21 insertions(+), 1 deletion(-) diff --git a/ares_cancel.3 b/ares_cancel.3 index ef10ec5..c34d8c5 100644 --- a/ares_cancel.3 +++ b/ares_cancel.3 @@ -28,7 +28,9 @@ name service channel identified by \fIchannel\fP. \fBares_cancel\fP invokes the callbacks for each pending query on the channel, passing a status of .BR ARES_ECANCELLED . These calls give the callbacks a chance to clean up any state which -might have been stored in their arguments. +might have been stored in their arguments. If a callback adds a new request +to the channel, that request will be cancelled as well, so that upon return +of \fBares_cancel\fP, no requests remain. .SH SEE ALSO .BR ares_init (3) .BR ares_destroy (3) diff --git a/ares_gethostbyaddr.3 b/ares_gethostbyaddr.3 index f589868..7727307 100644 --- a/ares_gethostbyaddr.3 +++ b/ares_gethostbyaddr.3 @@ -70,6 +70,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_gethostbyname.3 b/ares_gethostbyname.3 index a578a53..6b24ea4 100644 --- a/ares_gethostbyname.3 +++ b/ares_gethostbyname.3 @@ -74,6 +74,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_getnameinfo.3 b/ares_getnameinfo.3 index 7e4990c..d227606 100644 --- a/ares_getnameinfo.3 +++ b/ares_getnameinfo.3 @@ -109,6 +109,9 @@ was not found. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_query.3 b/ares_query.3 index 0c5df84..733fbc9 100644 --- a/ares_query.3 +++ b/ares_query.3 @@ -118,6 +118,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_search.3 b/ares_search.3 index 4184b00..2c85d20 100644 --- a/ares_search.3 +++ b/ares_search.3 @@ -119,6 +119,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel diff --git a/ares_send.3 b/ares_send.3 index 48d90ab..b89abfe 100644 --- a/ares_send.3 +++ b/ares_send.3 @@ -73,6 +73,9 @@ No name servers could be contacted. .B ARES_ENOMEM Memory was exhausted. .TP 19 +.B ARES_ECANCELLED +The query was cancelled. +.TP 19 .B ARES_EDESTRUCTION The name service channel .I channel -- 1.7.9.5
[PATCH 1/4] ares_cancel(): ensure cancellation of all requests
An invocation of ares_cancel() walks through the request list, calling the callbacks of all pending requests on a channel. Previously, if such a callback added a new request to the channel, the request list might not end up empty, causing an abort by assertion failure. The present commit ensures that all such newly added requests are cancelled immediately and make it never into request list. Thus, the crash is avoided, and it is made certain that upon return of ares_cancel(), there are no requests whatsoever on the channel. --- ares.h |1 + ares_cancel.c|6 ++ ares_gethostbyaddr.c |6 ++ ares_gethostbyname.c |6 ++ ares_getnameinfo.c |6 ++ ares_private.h |3 +++ ares_query.c |6 ++ ares_search.c|6 ++ ares_send.c |6 ++ 9 files changed, 46 insertions(+) diff --git a/ares.h b/ares.h index 9b3f376..8a9b995 100644 --- a/ares.h +++ b/ares.h @@ -140,6 +140,7 @@ extern "C" { #define ARES_FLAG_NOALIASES (1 << 6) #define ARES_FLAG_NOCHECKRESP (1 << 7) #define ARES_FLAG_EDNS (1 << 8) +/* #define ARES_FLAG_CANCELLING(1 << 9) */ /* defined in ares_private.h */ /* Option mask values */ #define ARES_OPT_FLAGS (1 << 0) diff --git a/ares_cancel.c b/ares_cancel.c index e5bb050..012fec1 100644 --- a/ares_cancel.c +++ b/ares_cancel.c @@ -30,6 +30,11 @@ void ares_cancel(ares_channel channel) struct list_node* list_node; int i; + if (channel->flags & ARES_FLAG_CANCELLING) + { +return; /* Already being cancelled */ + } + channel->flags |= ARES_FLAG_CANCELLING; list_head = &(channel->all_queries); for (list_node = list_head->next; list_node != list_head; ) { @@ -60,4 +65,5 @@ void ares_cancel(ares_channel channel) ares__close_sockets(channel, &channel->servers[i]); } } + channel->flags &= ~ARES_FLAG_CANCELLING; } diff --git a/ares_gethostbyaddr.c b/ares_gethostbyaddr.c index 85862e2..f77dafd 100644 --- a/ares_gethostbyaddr.c +++ b/ares_gethostbyaddr.c @@ -66,6 +66,12 @@ void ares_gethostbyaddr(ares_channel channel, const void *addr, int addrlen, { struct addr_query *aquery; + if (channel->flags & ARES_FLAG_CANCELLING) +{ + callback(arg, ARES_ECANCELLED, 0, NULL); + return; +} + if (family != AF_INET && family != AF_INET6) { callback(arg, ARES_ENOTIMP, 0, NULL); diff --git a/ares_gethostbyname.c b/ares_gethostbyname.c index 2b27b2e..12e452b 100644 --- a/ares_gethostbyname.c +++ b/ares_gethostbyname.c @@ -95,6 +95,12 @@ void ares_gethostbyname(ares_channel channel, const char *name, int family, return; } + if (channel->flags & ARES_FLAG_CANCELLING) +{ + callback(arg, ARES_ECANCELLED, 0, NULL); + return; +} + if (fake_hostent(name, family, callback, arg)) return; diff --git a/ares_getnameinfo.c b/ares_getnameinfo.c index 5b9f638..26521be 100644 --- a/ares_getnameinfo.c +++ b/ares_getnameinfo.c @@ -88,6 +88,12 @@ void ares_getnameinfo(ares_channel channel, const struct sockaddr *sa, struct nameinfo_query *niquery; unsigned int port = 0; + if (channel->flags & ARES_FLAG_CANCELLING) +{ + callback(arg, ARES_ECANCELLED, 0, NULL, NULL); + return; +} + /* Validate socket address family and length */ if ((sa->sa_family == AF_INET) && (salen == sizeof(struct sockaddr_in))) diff --git a/ares_private.h b/ares_private.h index ab5be5a..14b9ac2 100644 --- a/ares_private.h +++ b/ares_private.h @@ -113,6 +113,9 @@ #define EDNSFIXEDSZ11/* Size of EDNS header */ /* EDNS defines section **/ +#define ARES_FLAG_CANCELLING (1 << 9) /* flag indicating all requests + being cancelled on a channel */ + struct ares_addr { int family; union { diff --git a/ares_query.c b/ares_query.c index 4bc9c25..2e8949a 100644 --- a/ares_query.c +++ b/ares_query.c @@ -115,6 +115,12 @@ void ares_query(ares_channel channel, const char *name, int dnsclass, unsigned char *qbuf; int qlen, rd, status; + if (channel->flags & ARES_FLAG_CANCELLING) +{ + callback(arg, ARES_ECANCELLED, 0, NULL, 0); + return; +} + /* Compose the query. */ rd = !(channel->flags & ARES_FLAG_NORECURSE); status = ares_create_query(name, dnsclass, type, channel->next_id, rd, &qbuf, diff --git a/ares_search.c b/ares_search.c index ec07640..1d3e518 100644 --- a/ares_search.c +++ b/ares_search.c @@ -54,6 +54,12 @@ void ares_search(ares_channel channel, const char *name, int dnsclass, const char *p; int status, ndots; + if (channel->flags & ARES_FLAG_CANCELLING) +{ + callback(arg, ARES_ECANCELLED, 0, NULL, 0); + return; +} + /* If name only yields one domain to search, then we don't have * to keep extra state, so just do an ares_query(). */ diff --git a/ares_send.c b/ares_send.c index 1a450b1..81a4049 100644 --- a/ares_send.c +++ b/ares_send.c @@ -3
[PATCH 4/4] .gitignore: ignore patch files
This commit adds a line to .gitignore to the effect that patch files generated by 'git format-patch' are excluded from the repository. --- .gitignore |1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 19a0eae..0b1087a 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,4 @@ ares_timeout.pdf ares_version.pdf c-ares-*.tar.gz.asc ares_parse_mx_reply.pdf +/[0-9]*.patch -- 1.7.9.5
Cosmetic bugs in http://c-ares.haxx.se/ares_parse_a_reply.html and http://c-ares.haxx.se/ares_parse_aaaa_reply.html
Hi, in the HTML pages mentioned in the subject, there are spurious '\fB' sequences in the synopses. Possibly they have to be changed to '\fP' in the source man pages, but I don't know enough roff to say for sure. Best regards, Alexander -- Dr. Alexander Klauer Competence Centre for High Performance Computing Fraunhofer-Institut für Techno- und Wirtschaftsmathematik ITWM Fraunhofer-Platz 1 67663 Kaiserslautern Tel.: +49 631 31600-4335 Fax : +49 631 31600-5335 Email: alexander.kla...@itwm.fraunhofer.de
Re: [PATCH 2/3] library init: documentation update
On 03/15/2013 08:26 PM, Yang Tse wrote: On Fri, Mar 15, 2013, Alexander Klauer wrote: This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the new recursive behaviour. It might be better to use "Its reference counted initialize/deinitialize behavior," or something similar, instead of "Its recursive behavior," OK, I've updated the commit. Best regards, Alexander
[PATCH 2/3] library init: documentation update
This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the newly introduced reference counting of initializations and deinitializations. --- ares_library_cleanup.3 | 19 --- ares_library_init.3| 18 -- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/ares_library_cleanup.3 b/ares_library_cleanup.3 index 9fcf896..d60c378 100644 --- a/ares_library_cleanup.3 +++ b/ares_library_cleanup.3 @@ -31,11 +31,19 @@ The .B ares_library_cleanup function uninitializes the c-ares library, freeing all resources previously acquired by \fIares_library_init(3)\fP when the library -was initialized. +was initialized, provided there was only one single previous call to +\fIares_library_init(3)\fP. If there was more than one previous call to +\fIares_library_init(3)\fP, this function uninitializes the c-ares +library only if it is the call matching the call to +\fIares_library_init(3)\fP which initialized the library +(usually the very first call to \fIares_library_init(3)\fP). +Other calls to \fIares_library_cleanup(3)\fP have no effect other than +decrementing an internal counter. .PP This function must be called when the program using c-ares will no longer need any c-ares function. Once the program has called -\fIares_library_cleanup(3)\fP it shall not make any further call to any +\fIares_library_cleanup(3)\fP sufficiently often such that the +library is uninitialised, it shall not make any further call to any c-ares function. .PP This function does not cancel any pending c-ares lookups or requests @@ -54,7 +62,12 @@ the DllMain function. Doing so will produce deadlocks and other problems. .SH AVAILABILITY This function was first introduced in c-ares version 1.7.0 along with the definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_CLEANUP\fP as an -indication of the availability of this function. +indication of the availability of this function. Reference counting in +\fIares_library_init()\fP and \fIares_library_cleanup()\fP, which requires +calls to the former function to match calls to the latter, is present since +c-ares version 1.10.0. +Earlier versions would deinitialize the library on the first call +to \fIares_library_cleanup()\fP. .PP Since the introduction of this function, it is absolutely mandatory to call it for any Win32/64 program using c-ares. diff --git a/ares_library_init.3 b/ares_library_init.3 index b3efc18..797476b 100644 --- a/ares_library_init.3 +++ b/ares_library_init.3 @@ -33,13 +33,15 @@ function performs initializations internally required by the c-ares library that must take place before any other function provided by c-ares can be used in a program. .PP -This function must be called one time within the life of a program, +This function must be called at least once within the life of a program, before the program actually executes any other c-ares library function. -Initializations done by this function remain effective until a -call to \fIares_library_cleanup(3)\fP is performed. +Initializations done by this function remain effective until a number of +calls to \fIares_library_cleanup(3)\fP equal to the number of calls to +this function are performed. .PP -Successive calls to this function do nothing, only the first call done -when c-ares is in an uninitialized state is actually effective. +Successive calls to this function do nothing further, only the first +call done when c-ares is in an uninitialized state is actually +effective. .PP The .I flags @@ -77,7 +79,11 @@ non-zero error number will be returned to indicate the error. Except for .SH AVAILABILITY This function was first introduced in c-ares version 1.7.0 along with the definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_INIT\fP as an -indication of the availability of this function. +indication of the availability of this function. Its recursive behavior, +which requires a matching number of calls to \fIares_library_cleanup()\fP +in order to deinitialize the library, is present since c-ares version +1.10.0. Earlier versions would deinitialize the library on the first call +to \fIares_library_cleanup()\fP. .PP Since the introduction of this function it is absolutely mandatory to call it for any Win32/64 program using c-ares. -- 1.7.9.5
[PATCH 3/3] .gitignore: ignore patch files
This commit adds a line to .gitignore to the effect that patch files generated by 'git format-patch' are excluded from the repository. --- .gitignore |1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 19a0eae..0b1087a 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,4 @@ ares_timeout.pdf ares_version.pdf c-ares-*.tar.gz.asc ares_parse_mx_reply.pdf +/[0-9]*.patch -- 1.7.9.5
[PATCH 2/3] library init: documentation update
This commit updates the documentation of ares_library_init() and ares_library_cleanup() with regard to the new recursive behaviour. --- ares_library_cleanup.3 | 18 +++--- ares_library_init.3| 18 -- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/ares_library_cleanup.3 b/ares_library_cleanup.3 index 9fcf896..2071018 100644 --- a/ares_library_cleanup.3 +++ b/ares_library_cleanup.3 @@ -31,11 +31,19 @@ The .B ares_library_cleanup function uninitializes the c-ares library, freeing all resources previously acquired by \fIares_library_init(3)\fP when the library -was initialized. +was initialized, provided there was only one single previous call to +\fIares_library_init(3)\fP. If there was more than one previous call to +\fIares_library_init(3)\fP, this function uninitializes the c-ares +library only if it is the call matching the call to +\fIares_library_init(3)\fP which initialized the library +(usually the very first call to \fIares_library_init(3)\fP). +Other calls to \fIares_library_cleanup(3)\fP have no effect other than +decrementing an internal counter. .PP This function must be called when the program using c-ares will no longer need any c-ares function. Once the program has called -\fIares_library_cleanup(3)\fP it shall not make any further call to any +\fIares_library_cleanup(3)\fP sufficiently often such that the +library is uninitialised, it shall not make any further call to any c-ares function. .PP This function does not cancel any pending c-ares lookups or requests @@ -54,7 +62,11 @@ the DllMain function. Doing so will produce deadlocks and other problems. .SH AVAILABILITY This function was first introduced in c-ares version 1.7.0 along with the definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_CLEANUP\fP as an -indication of the availability of this function. +indication of the availability of this function. Its recursive behavior, +which requires calls to this functions to match calls to +\fIares_library_cleanup(3)\fP, is present since c-ares version 1.10.0. +Earlier versions would deinitialize the library on the first call +to \fIares_library_cleanup()\fP. .PP Since the introduction of this function, it is absolutely mandatory to call it for any Win32/64 program using c-ares. diff --git a/ares_library_init.3 b/ares_library_init.3 index b3efc18..797476b 100644 --- a/ares_library_init.3 +++ b/ares_library_init.3 @@ -33,13 +33,15 @@ function performs initializations internally required by the c-ares library that must take place before any other function provided by c-ares can be used in a program. .PP -This function must be called one time within the life of a program, +This function must be called at least once within the life of a program, before the program actually executes any other c-ares library function. -Initializations done by this function remain effective until a -call to \fIares_library_cleanup(3)\fP is performed. +Initializations done by this function remain effective until a number of +calls to \fIares_library_cleanup(3)\fP equal to the number of calls to +this function are performed. .PP -Successive calls to this function do nothing, only the first call done -when c-ares is in an uninitialized state is actually effective. +Successive calls to this function do nothing further, only the first +call done when c-ares is in an uninitialized state is actually +effective. .PP The .I flags @@ -77,7 +79,11 @@ non-zero error number will be returned to indicate the error. Except for .SH AVAILABILITY This function was first introduced in c-ares version 1.7.0 along with the definition of preprocessor symbol \fICARES_HAVE_ARES_LIBRARY_INIT\fP as an -indication of the availability of this function. +indication of the availability of this function. Its recursive behavior, +which requires a matching number of calls to \fIares_library_cleanup()\fP +in order to deinitialize the library, is present since c-ares version +1.10.0. Earlier versions would deinitialize the library on the first call +to \fIares_library_cleanup()\fP. .PP Since the introduction of this function it is absolutely mandatory to call it for any Win32/64 program using c-ares. -- 1.7.9.5
[PATCH] library init: be recursive
Previously, a single call to ares_library_cleanup() would deinitialise the c-ares library, regardless of how many times ares_library_init() was called. This behaviour may cause problems in programs linking two or more libraries which, in turn, use c-ares. The present commit fixes this problem, deinitializing the library only after a number of calls to ares_library_cleanup() matching the number of calls to ares_library_init(). --- ares_library_init.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ares_library_init.c b/ares_library_init.c index f0137a1..770e7c2 100644 --- a/ares_library_init.c +++ b/ares_library_init.c @@ -101,7 +101,10 @@ int ares_library_init(int flags) int res; if (ares_initialized) -return ARES_SUCCESS; +{ + ares_initialized++; + return ARES_SUCCESS; +} ares_initialized++; if (flags & ARES_LIB_INIT_WIN32) @@ -122,6 +125,8 @@ void ares_library_cleanup(void) if (!ares_initialized) return; ares_initialized--; + if (ares_initialized) +return; if (ares_init_flags & ARES_LIB_INIT_WIN32) ares_win32_cleanup(); -- 1.7.9.5