This page is a snapshot from the LWG issues list, see the Library Active Issues List for more information and the meaning of New status.

### 3594. `inout_ptr` — inconsistent `release()` in destructor

Section: 20.11.10 [inout.ptr.t] Status: New Submitter: JeanHeyd Meneide Opened: 2021-09-16 Last modified: 2021-10-30

Priority: 1

View all issues with New status.

Discussion:

More succinctly, the model for `std::out_ptr_t` and `std::inout_ptr_t` both have a conditional release in their destructors as part of their specification (20.11.8 [out.ptr.t] p8) (Option #2 below). But, if the wording is followed, they have an unconditional release in their constructor (Option #1 below). This is not exactly correct and can cause issues with double-free in `inout_ptr` in particular.

Consider a function `MyFunc` that sets `rawPtr` to `nullptr` when freeing an old value and deciding not to produce a new value, as shown below:

>     // Option #1:
>     auto uptr = std::make_unique<BYTE[]>(25);
>     auto rawPtr = uptr.get();
>     uptr.release(); // UNCONDITIONAL
>     MyFunc(&rawPtr);
>     If (rawPtr)
>     {
>       uptr.reset(rawPtr);
>     }

>     // Option #2:
>     auto uptr = std::make_unique<BYTE[]>(25);
>     auto rawPtr = uptr.get();
>     MyFunc(&rawPtr);
>     If (rawPtr)
>     {
>       uptr.release(); // CONDITIONAL
>       uptr.reset(rawPtr);
>     }

This is no problem if the implementation selected Option #1 (release in the constructor), but results in double-free if the implementation selected option #2 (release in the destructor).

As the paper author and after conferring with others, the intent was that the behavior was identical and whether a choice between the constructor or destructor is made. The reset should be unconditional, at least for `inout_ptr_t`. Suggested change for the `~inout_ptr_t` destructor text is to remove the "`if (p) { ... }`" wrapper from around the code in 20.11.10 [inout.ptr.t] p11.

[2021-09-24; Reflector poll]

Set priority to 1 after reflector poll.

Previous resolution [SUPERSEDED]:

> This wording is relative to N4892.

> 1. Modify 20.11.10 [inout.ptr.t] as indicated:

>    >     ~inout_ptr_t();

>    > > -9- Let `SP` be `POINTER_OF_OR(Smart, Pointer)` (20.10.1 [memory.general]).

>    > > -10- Let `release-statement` be `s.release()`; if an implementation does not call `s.release()` in the constructor. Otherwise, it is empty.

>    > > -11- Effects: Equivalent to:

>    > > 1. (11.1) —

>    > >    >     if (p) {
>    > >    >       apply([&](auto&&... args) {
>    > >    >       s = Smart( static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
>    > >    >     }

>    > >    if `is_pointer_v<Smart>` is `true`;

>    > > 2. (11.2) — otherwise,

>    > >    >     if (p) {
>    > >    >       apply([&](auto&&... args) {
>    > >    >       release-statement;
>    > >    >       s.reset(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
>    > >    >     }

>    > >    if the expression `s.reset(static_cast<SP>(p), std::forward<Args>(args)...)` is well-formed;

>    > > 3. (11.3) — otherwise,

>    > >    >     if (p) {
>    > >    >       apply([&](auto&&... args) {
>    > >    >       release-statement;
>    > >    >       s = Smart(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
>    > >    >     }

>    > >    if `is_constructible_v<Smart, SP, Args...>` is `true`;

>    > > 4. (11.4) — otherwise, the program is ill-formed.

[2021-10-28; JeanHeyd Meneide provides improved wording]

Proposed resolution:

This wording is relative to N4901.

1. Modify 20.11.10 [inout.ptr.t] as indicated:

   >     ~inout_ptr_t();

   > > -9- Let `SP` be `POINTER_OF_OR(Smart, Pointer)` (20.10.1 [memory.general]).

   > > -10- Let `release-statement` be `s.release()`; if an implementation does not call `s.release()` in the constructor. Otherwise, it is empty.

   > > -11- Effects: Equivalent to:

   > > 1. (11.1) —

   > >    >     if (p) {
   > >    >       apply([&](auto&&... args) {
   > >    >       s = Smart( static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
   > >    >     }

   > >    if `is_pointer_v<Smart>` is `true`;

   > > 2. (11.2) — otherwise,

   > >    >     release-statement;
   > >    >     if (p) {
   > >    >       apply([&](auto&&... args) {
   > >    >       release-statement;
   > >    >       s.reset(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
   > >    >     }

   > >    if the expression `s.reset(static_cast<SP>(p), std::forward<Args>(args)...)` is well-formed;

   > > 3. (11.3) — otherwise,

   > >    >     release-statement;
   > >    >     if (p) {
   > >    >       apply([&](auto&&... args) {
   > >    >       release-statement;
   > >    >       s = Smart(static_cast<SP>(p), std::forward<Args>(args)...); }, std::move(a));
   > >    >     }

   > >    if `is_constructible_v<Smart, SP, Args...>` is `true`;

   > > 4. (11.4) — otherwise, the program is ill-formed.