Boost logo

Boost :

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2019-06-25 22:05:56

On 6/25/19 11:02 PM, JeanHeyd Meneide wrote:
> On Tue, Jun 25, 2019 at 6:29 AM Andrey Semashev via Boost
> <boost_at_[hidden] <mailto:boost_at_[hidden]>> wrote:
> > I think, an optional optimization is not an adequate solution -
> it has
> > observable difference in behavior. The library has to give the
> user a
> > guaranteed behavior that allows him to write code that has some
> > guaranteed effect (e.g. not terminating the program in case of
> > exception). If inout_ptr cannot be implemented on those terms
> then you
> > have to think what can be reasonably done to make it work. For
> example,
> > you could add a requirement that reset() doesn't throw. Or that you
> > don't call reset() if the returned pointer didn't change.
> Or, you don't do anything at all in the destructor if an exception
> is in
> flight.
>      The problematic case is when `reset()` does throw, like with
> shared_ptr. I don't think it's useful or fair to impose the
> comparatively high overhead of
> std::uncaught_exceptions()/std::uncaught_exception() checking in the
> constructor and destructor to prevent this class of errors.

The overhead would be close to a library function call, since the
functions return a piece of the current thread state. It's not zero, but
it's not expensive either.

> The most
> graceful thing that can happen is an exception is thrown from the
> .reset(), and being the first one gets caught by the scope just above.

You mean, catch and suppress any exceptions thrown by reset() and assume
it had freed the pointer before throwing? First, this is a requirement
on reset(), which has to be documented. shared_ptr notwithstanding.

Second, I don't think suppressing the exception when there is no other
exception in flight is an expected behavior. If the code involving a
function call with out_ptr argument completed without exceptions, any
user would assume that all is fine, and the returned pointer is properly
saved in the resulting smart pointer. I think, you will still have to
check whether there is an outer exception in flight and let the
exception from reset() propagate only if there isn't one.

> Cleverness by providing the "guarantee" of not calling .reset() if an
> exception is in progress means actually leaking memory if the .reset()
> call fails: shared_ptr and friends all guarantee in their
> .reset/constructor calls will call delete for you if they toss an
> exception (

Not calling reset() on outer exception makes sense if you require that
the function that threw it either didn't allocate or freed the memory
before the exception propagated out of the function. In other words, if
the function fails, it must clean up the mess after itself before
returning, exception or not. This is not an unreasonable requirement,
and in fact, this is the only behavior I consider reasonable in the
context of raw pointers.

Out of all of the possible solutions brought up so far, this one seems
the most logical one to me. I seem to remember some real APIs that
modify the out pointers on failure, but even then they free any
allocated memory before returning. Meaning that analyzing or using the
out pointers on failure is not always safe or useful.

> This
> means even if you terminate you have memory safety:

I don't care about memory leaks if std::terminate() is called. :)

>      I am very certain that specifying anything more than conditional
> noexcept is not a good idea.

This simply means you're not going to support throwing reset(), because
that means std::terminate(). At this point you might as well document
this as a requirement and enforce in the implementation. No need for a
conditional noexcept.

That would make shared_ptr unsupported. Which might be an acceptable
loss, given that C-style APIs cannot return a shared_ptr (meaning a
pointer, which is owned by another shared_ptr). From the caller's
standpoint, the returned pointer is unique, so the caller should be
using unique_ptr to receive the pointer. He may later use that
unique_ptr to initialize a shared_ptr, but that would be out of
Boost.OutPtr scope.

Boost list run by bdawes at, gregod at, cpdaniel at, john at