高效优雅地返回已放置的unique_ptr

4
我发现(感谢StackOverflow评论)我的代码存在安全漏洞:
std::vector<std::unique_ptr<Item>> items;

template<class... TS> Item& create(TS&&... mArgs)
{
    auto item(new Item(std::forward<TS>(mArgs)...);
    items.emplace_back(item); // Possible exception and memory leak
    return *item;
}

基本上,如果emplace_back抛出异常,使用原始的newItem分配内存可能会导致内存泄漏。
解决方案是从不使用原始的new,而是在方法体中使用std::unique_ptr
std::vector<std::unique_ptr<Item>> items;

template<class... TS> Item& create(TS&&... mArgs)
{
    auto item(std::make_unique<Item>(std::forward<TS>(mArgs)...);
    items.emplace_back(std::move(item));
    return *item; // `item` was moved, this is invalid!
}

如您所见,返回item是无效的,因为我必须使用std::move移动item以将其放置到items容器中。

我想不出需要在额外变量中存储item地址的解决方案。然而,原始(有缺陷的)解决方案非常简洁易读。

是否有更优雅的方法返回已移动以用于容器插入的std::unique_ptr


3
返回 *items.back(); - Jarod42
如果那个 items 向量突然消失了呢? - Joker_vD
1
顺便提一下,注意代码只提供基本的异常保证:如果 emplace_back 抛出异常,参数将已经被移动。 - avakar
@avakar,有没有解决这个问题的方法? - Vittorio Romeo
@avakar @VittorioRomeo 我认为这不是问题,传递进去的临时变量本来就会被销毁。至于拷贝,只有当某些参数的拷贝构造函数是非const时才会有问题。 - Joker_vD
显示剩余2条评论
3个回答

8
你可以这样写:

你可以写:

template<class... TS>
Item& create(TS&&... mArgs)
{
    items.emplace_back(std::make_unique<Item>(std::forward<TS>(mArgs)...));
    return *items.back();
}

2
在emplace操作之前缓存引用是一种更普遍适用的选项(例如,对于非向量容器):
template<class... TS> Item& create(TS&&... mArgs)
{
    auto item = std::make_unique<Item>(std::forward<TS>(mArgs)...);
    auto& foo = *item;
    items.emplace_back(std::move(item));
    return foo; // This *is* valid.
}

虽然我认为如果 emplace 抛出异常仍可能会出现问题,但 Andrej 最近有一篇文章建议先调用 .reserve()。 - NoSenseEtAl
@NoSenseEtAl 这基本上是我们在 OP 的评论中讨论的问题 - 尽管没有人真正明确表达。这足以提供基本的异常安全保证:如果 emplace_back 抛出异常,则作为 rvalue 传递给 create 的参数将保留在移动状态下,并且其内部将被摧毁。如果需要提供强异常安全保证,则应该在任何其他操作之前,特别是潜在的将参数移动到 unique_ptr 中之前,预留向量中的空间 items.reserve(items.size() + 1) - Casey
1
更正:由于无法确保Item构造函数成功或不修改mArgs,因此此函数无法提供强异常安全性保证。您可以保证(a)如果抛出异常,则容器不会改变,以及(b)除非Item构造函数抛出异常,否则参数不会被修改。如果强保证至关重要,则必须修改该函数以接受unique_ptr参数并将其简单地移动到向量中。 - Casey

2

您的问题标记为C++11,其他建议使用make_unique的答案未提及它是C++14的功能。我认为这种C++11方法也解决了泄漏问题。

#include <vector>
#include <memory>
#include <utility>
#include <iostream>

struct Item
{
   int a, b;
   Item(int aa, int bb) : a{aa}, b{bb} { }
};

static std::vector<std::unique_ptr<Item>> items;

template <class... Ts> Item& create(Ts&&... args)
{
    items.emplace_back(std::unique_ptr<Item>{new Item(std::forward<Ts>(args)...)});
    return *items.back();
}

int main()
{
    Item& x = create(1, 2);
    std::cout << "( " << x.a << ", " << x.b << " )" << std::endl;
}

这应该是安全的,因为只有在已经构造了一个unique_ptr<Item>之后才能调用emplace_back(),所以即使emplace_back()抛出异常,你的Item已经由unique_ptr管理。

create 函数中,这个 unique_ptr<Item> 是在哪里构造的?我没有看到它。你是不是想写成 items.emplace_back(std::unique_ptr<Item>{new Item(std::forward<Ts>(args)...)}); - Casey
我想当时我认为unique_ptr<Item>必须从emplace_back的参数中隐式构造,但现在我明白那是不对的。是的,你的建议看起来更好。 - zmb

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接