[getdns-api] getdns_return_t for destroy methods

[Paul Hoffman]

On Mar 8, 2014, at 7:03 PM, Goyal, Neel <ngoyal at verisign.com> wrote:

> Here is some code that illustrates the particular problem (error checking omitted for brevity):  https://gist.github.com/ngoyal/9436881 - getdns_context_destroy can be called on the same context multiple times before the first call to getdns_context_destroy returns because the destroy method must cancel all outstanding requests.  Canceling those requests ends up calling the same callback which in turns calls destroy.  
> 
> This isn't unmanageable, but managing it leads to hairy code as it has to deal with the re-entrant nature of the problem.  Without proper management, this could lead to some serious problems as the pointers "dangle" and point to invalid memory - NULL checks fail in that case and seg faults are bound to occur.  Remember that in C, calling free(ptr); does not mean that ptr == NULL when it returns.  Calling free(ptr) again will end up causing undefined behavior in the program (most likely a crash).
> 
> In the gist code, the context_destroy method that is called within the callback could return an error code indicating that it cannot be destroyed while firing a callback.  In the common case, most folks probably won't destroy a context within a callback, but as implementors, we must ensure we work without crashing in as many cases we can think of. 

I'm feeling dense. Why are you destroying the context *inside* the callback? For your example, why wouldn't you destroy it after all the calls have run, even if unsuccessfully?

That is, it feels like you want a return value because you want to destroy the context in the wrong place. I'm inclined to say "don't do that" unless you can show why it is a better place to do it.

--Paul Hoffman