C++ – 在迭代std::list并删除元素时出现错误

huangapple go评论72阅读模式
英文:

C++ - Error while iterating std::list and deleting element

问题

以下是您要翻译的代码部分:

This is my code:

    std::list<GameMatch*> games_;

    void GameMatchManager::ClearEndedGames()
    {
        if(games_.empty())
            return;
    
        auto it = games_.begin();
        while (it != games_.end())
        {
            auto game = *it;
            if(game->MatchEnded())
            {
                games_.erase(it);
                game->Destroy();
            }
            ++it;
        }
    }

    void GameMatch::Destroy()
    {
        std::cout << "Destoying " << GetMatchId() << std::endl;
        delete this;
    }

This is the error I am getting:

[![enter image description here][1]][1]

Why am I getting this error in first place when at the top i do `if(games.empty()) return;` ?

Shouldn't that stop the iteration of `games_` list ?
Why am I getting this error and how can I fix it?

希望这有助于您解决问题。

英文:

This is my code:

std::list&lt;GameMatch*&gt; games_;

void GameMatchManager::ClearEndedGames()
{
    if(games_.empty())
        return;

    auto it = games_.begin();
    while (it != games_.end())
    {
        auto game = *it;
        if(game-&gt;MatchEnded())
        {
            games_.erase(it);
            game-&gt;Destroy();
        }
        ++it;
    }
}

void GameMatch::Destroy()
{
    std::cout &lt;&lt; &quot;Destoying &quot; &lt;&lt; GetMatchId() &lt;&lt; std::endl;
    delete this;
}

This is the error I am getting:

C++ – 在迭代std::list并删除元素时出现错误

Why am I getting this error in first place when at the top i do if(games.empty()) return; ?

Shouldn't that stop the iteration of games_ list ?
Why am I getting this error and how can I fix it?

答案1

得分: 1

明显的问题:您正在使用无效的迭代器。当您从std::list中删除一个元素时,指向其他元素的迭代器仍然有效,但被删除的元素的迭代器处于未定义状态。一个可能的解决方案是在删除之前先迭代一步:

template <typename List>
void ConsumeList(List&& list) {
  auto last_it = list.begin();
  if (last_it != list.end()) {
    for (auto it = std::next(last_it); it != list.end(); ++it) {
      list.erase(last_it);  // 此处 last_it 变为无效状态。
      last_it = it;         // 没关系,我们重新使其有效。
    }
    list.erase(last_it);    // 不要忘记这个!
  }
}

其他注意事项,大部分与问题无关:

  1. 避免使用Destroy()等方法;它可能执行的是应该由~GameMatch()执行的操作,但没有来自编译器的强有关的RAII保证。优先选择RAII而不是“自制”资源管理。
  2. 不要使用裸指针。您几乎永远不需要它们。
  3. 最好根本不直接分配对象(使用智能指针或其他方式)。让STL容器处理分配。直接嵌入对象(std::list<GameMatch>而不是std::list<GameMatch*>)。指针跳跃越少,效率越高。

这是ConsumeList()的示例,这次在一个可构建和可运行上下文中的代码:

#include <iostream>
#include <iterator>
#include <list>
#include <memory>
#include <string>
#include <string_view>
#include <utility>

namespace {

struct GameMatch {
  GameMatch(std::string_view match_id) : match_id_{match_id} {}
  std::string_view GetMatchId() const { return match_id_; }
  ~GameMatch() { std::cout << "Destoying " << GetMatchId() << std::endl; }

 private:
  const std::string match_id_;
};

template <typename List>
void ConsumeList(List&& list) {
  auto last_it = list.begin();
  if (last_it != list.end()) {
    for (auto it = std::next(last_it); it != list

<details>
<summary>英文:</summary>

The obvious problem: You are using an invalid iterator. When you delete an element from a `std::list`, iterators pointing at **other** elements remain valid, but the deleted elements iterator is in an undefined state. A possible solution is to iterate one step ahead of deletions:

```c++
template &lt;typename List&gt;
void ConsumeList(List&amp;&amp; list) {
  auto last_it = list.begin();
  if (last_it != list.end()) {
    for (auto it = std::next(last_it); it != list.end(); ++it) {
      list.erase(last_it);  // last_it becomes invalid here.
      last_it = it;         // Never mind, we make it valid again.
    }
    list.erase(last_it);    // Don&#39;t forget this!
  }
}

Other notes, mostly unrelated to the problem:

  1. Avoid methods such as Destroy(); it is likely doing something that ~GameMatch() should be doing instead, but without strong RAII-related guarantees from the compiler. Prefer RAII over “homebrew” resource management.
  2. Do not use raw pointers. You almost never(*) need them.
    (*) The only exception is when you implement low-level linked data structures. Then (and only then) it makes sense to use raw pointers.
  3. Preferably, do not allocate objects directly (using smart pointers or otherwise) at all. Instead, let STL containers handle allocation. Embed your objects directly (std::list&lt;GameMatch&gt; instead of std::list&lt;GameMatch*&gt;). The fewer pointer jumps, the better efficiency you get.

Here’s ConsumeList() once again, this time in a buildable and runnable context:

#include &lt;iostream&gt;
#include &lt;iterator&gt;
#include &lt;list&gt;
#include &lt;memory&gt;
#include &lt;string&gt;
#include &lt;string_view&gt;
#include &lt;utility&gt;

namespace {

struct GameMatch {
  GameMatch(std::string_view match_id) : match_id_{match_id} {}
  std::string_view GetMatchId() const { return match_id_; }
  ~GameMatch() { std::cout &lt;&lt; &quot;Destoying &quot; &lt;&lt; GetMatchId() &lt;&lt; std::endl; }

 private:
  const std::string match_id_;
};

template &lt;typename List&gt;
void ConsumeList(List&amp;&amp; list) {
  auto last_it = list.begin();
  if (last_it != list.end()) {
    for (auto it = std::next(last_it); it != list.end(); ++it) {
      list.erase(last_it);
      last_it = it;
    }
    list.erase(last_it);
  }
}

}  // namespace

int main() {
  std::list&lt;GameMatch&gt; embedded_list;
  embedded_list.emplace_back(&quot;Embedded Match A&quot;);
  embedded_list.emplace_back(&quot;Embedded Match B&quot;);
  embedded_list.emplace_back(&quot;Embedded Match C&quot;);

  std::list&lt;std::unique_ptr&lt;GameMatch&gt;&gt; pointer_list;
  pointer_list.emplace_back(std::make_unique&lt;GameMatch&gt;(&quot;Allocated Match A&quot;));
  pointer_list.emplace_back(std::make_unique&lt;GameMatch&gt;(&quot;Allocated Match B&quot;));
  pointer_list.emplace_back(std::make_unique&lt;GameMatch&gt;(&quot;Allocated Match C&quot;));
  
  ConsumeList(std::move(embedded_list));
  ConsumeList(std::move(pointer_list));
}

Notice that forgetting to run ConsumeList() will not cause any memory leaks at the end and all list elements will be properly deallocated. That is the key advantage of using RAII and avoiding raw pointers.

huangapple
  • 本文由 发表于 2023年6月16日 01:56:26
  • 转载请务必保留本文链接:https://go.coder-hub.com/76484335.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定