There's a lot of waxing poetical about C API designs below, skip to
the "-------------------------------" if you want to get to what a
more effective change would be.

Reading this over, I think at the core there's effectively 2 different
styles of C API that take in-and-(maybe-)out pointers, where the
"release always" approach is either good or bad in a mutually
exclusive way. Let's elaborate using 2 implementations of MyFunc, one
that shows the behavior you have here and the other that is the kind I
(one of the proposal authors for out_ptr/inout_ptr) encountered more
often (always?) in the wild:

// Implementation (1)
void MyFunc(BYTE** p_bytes) {
      free(*ptr);
     BYTE* new_ptr = new BYTE[42]();
      if (new_ptr) {
         *p_bytes = new_ptr;
     }
     else {
          *p_bytes = new_ptr;
     }
}

// Implementation (2)
void MyFunc(BYTE** p_bytes) {
     BYTE* new_ptr = new BYTE[42]();
     if (new_ptr) {
          delete(*ptr);
         *p_bytes = new_ptr;
     }
     else {
          // literally nothing
     }
}

Let us consider always calling .release(), unconditionally, in the
constructor or the destructor of an inout_ptr_t. Here are the 2 things
that happen:

— For (1), it works as outlined in your post. There is no double-free,
because the pointer is unconditionally deleted by the C function. This
is good and solves the presented problem.

— For (2), it works to the opposite. This C API does not do anything
with the input if it cannot guarantee that it can replace the value.
Calling .release() unconditionally here results in the opposite bug of
a dangling resource.

What complicates matters more is that both approaches are very valid
to designing a C API. For example, (1) is a good implementation for
certain scenarios and for resource-constrained environments. Deleting
the resource always means that, in a resource-constrained environment,
you have a greater chance that the next re-creation of the resource is
more successful. This does, unfortunately, require making sure that
there is some sentinel value / error value communicated to the user
about this decision (e.g., setting the pointer to NULL).

(2) is a good implementation for systems meant to be more "inert" or
"atomic" in their changes. It guarantees that nothing happens and
everything is rolled back if it cannot guarantee that the replacement
happens. Either the in-out portion of the operation happens, or it
happens not at all. (This also matches the ideals behind exception
safety: for example, if "new" throws in either (1) or (2), the (2)
implementation is safer. In a C Environment, that's never supposed to
happen, but there are many implementations which
wink-wink-hint-hint-nudge-nudge and let you throw an exception or two
through C API calls anyways.)

-------------------------------
-------------------------------

Given that making these changes effectively throws one design under
the bus, I think this might be evidence that one of a few things might
need to happen (and I'd be willing to do the legwork to make sure this
happens, since it was my effort to put it in the standard after all!)

A. Add additional tracking to std::inout_ptr. This means holding onto
the "old" value and, instead of comparing against "nullptr", comparing
against the old value and letting that be the trigger for calling
.reset() (or equivalent). This covers (should cover?) the
always-delete case AND the delete-and-reset-or-do-nothing case. This
comes at the cost of additional performance, which I am none too sure
some of the original instigators of this proposal would be happy about
since one of the primary reasons was putting it in the standard so the
standard pointers had a chance to "directly reseat the pointer". I
need to think more about whether the direct re-seating optimization is
unaffected by this, and run some more tests after the coffee hits.

B. Create a third type that carefully captures the semantic difference
between "I unconditionally delete my pointers" and "I only
conditionally delete if the operation I am doing is a success". I'd
note the nomenclature for this might actually already be in place:
out_ptr only handles cases where a pointer is coming out. inout_ptr
only handles the cases where a pointer goes in AND comes out changed.
in_ptr would handle a pointer that goes in and is unconditionally
deleted, and only MAYBE comes out the other side. (I think people
would object to this kind of framing and the name in_ptr, but it is a
start. Perhaps the name could be in_maybe_out_ptr, or something
equally verbose). This approach retains performance no matter what,
but puts the burden of knowing the "kind" of C API on the end-user.

C. (This one needs to happen regardless.) I need to try to refine the
wording around how .release() is called in the constructor. The ideal
was to prevent the user from observing that .release() is called until
the destructor happens, effectively leaving the state of the original
smart pointer in flux until after the destructor.

I'm not sure which path of (A) or (B) is best. Given the example above
the marquee lines, I think that there are 2 different semantics here
and I am unsure of how mixing both kinds in inout_ptr may impact
speed. (It ultimately sounds like a better user experience, because
someone having to know what very well may be a secret internal
implementation detail to differentiate between one of 3 different
out_ptr / inout_ptr / in{something}_ptr is... tough.)

In either case, thank you for catching this, Doug!! I've got some
implementation and options to start benchmarking and testing, and some
people to e-mail! I'll make sure this gets fixed before ship.

Sincerely,
JeanHeyd