microsoft / microsoft/wil

Cleanup problems with `unique_*_ptr<T[]>` and `unique_*_array_ptr<T>`

Open
#242 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C++
Stars
3k
Forks
300
Avg merge
19h 12m
Merged PRs (30d)
1

Description

struct HasDtor {
    wistd::unique_ptr<uint32_t> foo;
};

// Works correctly; unique_ptr<T[]> uses delete[]
wistd::unique_ptr works<HasDtor[]>(new HasDtor[3]);

// Correctly fails to compile - wil::unique_*_ptr<T[]>(size_t) won't accept
// types with nontrivial destructors
auto oops = wil::make_unique_cotaskmem<HasDtor[]>(3);

// Compiles, but leaks
wil::unique_cotaskmem_ptr<HasDtor[]> oops2;
oops2.reset(reinterpret_cast<HasDtor*>(::CoTaskMemAlloc(sizeof(HasDtor) * 3)));
oops2[0].foo.reset(new uint32_t(6));

Both wil::unique_hlocal_ptr and wil::unique_cotaskmem_ptr are both using _ = wistd::unique_ptr<T, that_deleter>. Probably need to add some deduction magic to that which fails when the T is a T[], as it's probably always unsafe to use a type with a nontrivial destructor in this manner.

Similarly, the tantalizingly named wil::unique_*_array_ptr<T> which acts like an array with pointer-and-length compiles but skips the destructors:

wil::unique_cotaskmem_array_ptr<HasDtor> oops3;
withDtor.reset(reinterpret_cast<HasDtor*>(::CoTaskMemAlloc(sizeof(HasDtor) * 3)), 3);
oops3.reset();

That's because the default element-deleter for the type is just "do nothing"

struct empty_deleter {
    template <typename T> void operator()(_Pre_opt_valid_ _Frees_ptr_opt_ T) const {
    }
};
template <class T>
struct element_traits {
    typedef empty_deleter deleter;
    typedef T type;
};
template <typename T, typename ArrayDeleter>
using unique_array_ptr = unique_any_array_ptr<
    typename details::element_traits<T>::type, ArrayDeleter, 
    typename details::element_traits<T>::deleter>;

There's an attempt to rectify this by providing default cleanup for things that derive from unique_any_t<Q> and com_ptr_t<Q> by calling their dtors directly.

We can probably make this generic by having empty_deleter call ~T() which would correctly destroy complex types with dtors. That does have a bit of compat challenge, in case someone already knew about this defect and worked around it by manual destruction of the contained values.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the unique_ptr, unique_array_ptr, empty_deleter, and element_traits entry points described in the issue, including the existing special cases for unique_any_t and com_ptr_t. Determine safe destructor and compatibility behavior for nontrivial element types, then verify that the cleanup paths no longer skip destructors without breaking existing manual-destruction workarounds.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.