[getdns-api] getdns_return_t for destroy methods

[Goyal, Neel]

Not dense at all!  In fact, you're right on point.  Basically, a return code will enforce our ability to tell them "don't do that".  We can't control how a user is going to use our API, or when they'll make the necessary calls, so we just want to cover our bases and let them know they're doing something wrong in a way that doesn't result in a crash.
________________________________________
From: Paul Hoffman [paul.hoffman at vpnc.org]
Sent: Saturday, March 08, 2014 4:23 PM
To: Goyal, Neel
Cc: getdns-api at vpnc.org
Subject: Re: [getdns-api] getdns_return_t for destroy methods

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