Hold shared pointer to ensure lifetime

Author: tturocy

src/games/game.h

@@ -967,122 +967,66 @@ class GameRep : public std::enable_shared_from_this<GameRep> {
   virtual void BuildComputedValues() const {}
 };
 
-#include <assert.h>
-
 class GameRep::Infosets {
 public:
   class iterator {
   public:
-    using iterator_category = std::forward_iterator_tag; // diagnostic: proper category
+    using iterator_category = std::forward_iterator_tag;
     using value_type = GameInfoset;
     using difference_type = std::ptrdiff_t;
-    using pointer = value_type *;
-    using reference = value_type &;
+    using pointer = value_type;
+    using reference = value_type;
 
     using outer_iter = std::vector<std::shared_ptr<GamePlayerRep>>::const_iterator;
     using inner_iter = std::vector<std::shared_ptr<GameInfosetRep>>::const_iterator;
 
-    iterator() : owner_id_(0), players_(nullptr) {}
+    iterator() = default;
 
-    iterator(outer_iter outer, outer_iter outer_end,
-             const std::vector<std::shared_ptr<GamePlayerRep>> *players, std::size_t owner_id)
-      : outer_(outer), outer_end_(outer_end), players_(players), owner_id_(owner_id)
+    iterator(std::shared_ptr<GameRep> game, outer_iter outer, outer_iter outer_end)
+      : game_(std::move(game)), outer_(outer), outer_end_(outer_end)
     {
-      // ------ DIAGNOSTICS ----------------------------------------------------
-
-      // Detect dangling players pointer
-      assert(players_ != nullptr && "Infosets iterator constructed with null players pointer.");
-
-      // Detect begin()==end() but outer mismatch
-      if (outer_ == outer_end_) {
-        assert(inner_ == inner_end_ && "Inner iterators must be empty for end iterator.");
-      }
-
       if (outer_ != outer_end_) {
         init_inner();
         satisfy_invariant();
       }
     }
 
-    // -------------------------------------------------------------------------
-    // Dereference
-    // -------------------------------------------------------------------------
     value_type operator*() const
     {
-      assert(players_ != nullptr);
-      assert(!is_end() && "Dereferencing end iterator!");
-
-      // Check many possible UB causes
-      assert(inner_ != inner_end_ && "Inner iterator invalid during dereference.");
-      assert(valid_owner() &&
-             "Iterator used after view destruction or copying from different view.");
-
-      // Additional optional check: ensure GameInfoset is safe to construct
-      // You can instrument your GameInfoset(value_type) here to assert its invariants.
-
-      return *inner_;
+      return GameInfoset(*inner_); // GameObjectPtr constructor
     }
 
-    value_type operator->() const { return operator*(); }
+    value_type operator->() const { return GameInfoset(*inner_); }
 
-    // -------------------------------------------------------------------------
-    // Increment
-    // -------------------------------------------------------------------------
     iterator &operator++()
     {
-      assert(players_ != nullptr);
-      assert(!is_end() && "Incrementing end iterator!");
-
       ++inner_;
       satisfy_invariant();
       return *this;
     }
 
-    // -------------------------------------------------------------------------
-    // Equality / Inequality
-    // -------------------------------------------------------------------------
     bool operator==(const iterator &other) const
     {
-      // Detect comparing iterators from different views
-      if (owner_id_ != other.owner_id_) {
-        assert(false && "Comparing iterators from different Infosets views!");
-      }
-
-      // Both end iterators must compare equal
-      if (is_end() && other.is_end()) {
+      // Compare end iterators correctly
+      if (outer_ == outer_end_ && other.outer_ == other.outer_end_) {
         return true;
       }
 
-      // Detect illegal comparisons
-      assert(players_ == other.players_ &&
-             "Comparing iterators belonging to different container instances!");
-
       return outer_ == other.outer_ && inner_ == other.inner_;
     }
 
     bool operator!=(const iterator &other) const { return !(*this == other); }
 
   private:
-    // -------------------------------------------------------------------------
-    // Helpers
-    // -------------------------------------------------------------------------
     bool is_end() const { return outer_ == outer_end_; }
 
-    bool valid_owner() const { return players_ != nullptr && owner_id_ != 0; }
-
-    // Initialize inner iter safely
     void init_inner()
     {
-      const auto &playerPtr = *outer_;
-
-      // Check for null shared_ptr (VERY COMMON SOURCE OF UB)
-      assert(playerPtr && "Null shared_ptr<GamePlayerRep> inside m_players!");
-
-      inner_ = playerPtr->m_infosets.begin();
-      inner_end_ = playerPtr->m_infosets.end();
+      const auto &player = *outer_;
+      inner_ = player->m_infosets.begin();
+      inner_end_ = player->m_infosets.end();
     }
 
-    // Advance to next non-empty inner range
     void satisfy_invariant()
     {
       while (!is_end() && inner_ == inner_end_) {
@@ -1093,48 +1037,34 @@ class GameRep::Infosets {
       }
     }
 
-    // -------------------------------------------------------------------------
-    // Data members
-    // -------------------------------------------------------------------------
+    // HOLD GAME ALIVE
+    std::shared_ptr<GameRep> game_;
+
     outer_iter outer_{};
     outer_iter outer_end_{};
     inner_iter inner_{};
     inner_iter inner_end_{};
-
-    const std::vector<std::shared_ptr<GamePlayerRep>> *players_ = nullptr;
-
-    // Unique stamp to detect view lifetime/dangling iterator
-    std::size_t owner_id_ = 0;
   };
 
-  // -------------------------------------------------------------------------
-  // View interface
-  // -------------------------------------------------------------------------
-  iterator begin() const
+  // Construct a view storing a strong reference to the GameRep
+  explicit Infosets(std::shared_ptr<GameRep> game)
+    : game_(std::move(game)), players_(&game_->m_players)
   {
-    return iterator(players_->begin(), players_->end(), players_, owner_id_);
   }
 
-  iterator end() const { return iterator(players_->end(), players_->end(), players_, owner_id_); }
+  iterator begin() const { return iterator(game_, players_->begin(), players_->end()); }
 
-  explicit Infosets(const GameRep *game) : players_(&game->m_players)
-  {
-    assert(game != nullptr);
-    owner_id_ = next_owner_id(); // unique ID for detecting illegal comparisons
-  }
+  iterator end() const { return iterator(game_, players_->end(), players_->end()); }
 
 private:
+  std::shared_ptr<GameRep> game_;
   const std::vector<std::shared_ptr<GamePlayerRep>> *players_;
-  std::size_t owner_id_;
-
-  static std::size_t next_owner_id()
-  {
-    static std::size_t id = 1;
-    return id++;
-  }
 };
 
-inline GameRep::Infosets GameRep::GetInfosets() const { return Infosets(this); }
+inline GameRep::Infosets GameRep::GetInfosets() const
+{
+  return Infosets(std::const_pointer_cast<GameRep>(this->shared_from_this()));
+}
 
 //=======================================================================
 //          Inline members of game representation classes

src/games/gametree.h

@@ -127,7 +127,10 @@ class GameTreeRep : public GameExplicitRep {
   /// Returns the iset'th information set in the game (numbered globally)
   GameInfoset GetInfoset(int iset) const override;
   /// Returns the set of information sets in the game
-  Infosets GetInfosets() const override { return Infosets(this); }
+  Infosets GetInfosets() const override
+  {
+    return Infosets(std::const_pointer_cast<GameRep>(this->shared_from_this()));
+  }
   /// Sort the information sets for each player in a canonical order
   void SortInfosets() override;
   /// Returns the set of actions taken by the infoset's owner before reaching this infoset