Fix elusive memory corruption bug

Summary:
A corner case meant that a tracker that was in multiple lists that was ReadyForDestruction(), would get destroyed, and one of the tracker pointers would remain in the other list. Eventually, when that tracker was checked to see if it was ReadyForDestruction() there would be heap-after-use.

Solution: Don't allow a ConnectionTracker to live in two lists. Do this by keeping a back-pointer so we can immediately move a tracker when its protocol changes. The invariant is now that a tracker lives only in one list.

Test Plan: New fuzz test on ConnTrackersManager that triggers the bug (and is now fixed). Also testing on GKE...so keep fingers crossed.

Reviewers: yzhao, jps, #engineering

Reviewed By: yzhao, jps, #engineering

JIRA Issues: PP-2400

Differential Revision: https://phab.corp.pixielabs.ai/D7247

GitOrigin-RevId: 972520ec7429d058fcb33dec2207513fa778db4d

src/stirling/source_connectors/socket_tracer/BUILD.bazel

@@ -66,6 +66,16 @@ pl_cc_test(
     ],
 )
 
+pl_cc_test(
+    name = "conn_trackers_manager_test",
+    timeout = "moderate",
+    srcs = ["conn_trackers_manager_test.cc"],
+    deps = [
+        ":cc_library",
+        "//src/stirling/source_connectors/socket_tracer/testing:cc_library",
+    ],
+)
+
 pl_cc_test(
     name = "connection_tracker_http2_test",
     srcs = ["connection_tracker_http2_test.cc"],

src/stirling/source_connectors/socket_tracer/conn_trackers_manager.cc

@@ -15,17 +15,45 @@ uint64_t GetConnMapKey(uint32_t pid, uint32_t fd) {
 
 }  // namespace
 
-void ConnTrackersManager::NotifyProtocolChange(ConnectionTracker* tracker) {
+void ConnTrackersManager::UpdateProtocol(pl::stirling::ConnectionTracker* tracker,
+                                         std::optional<TrafficProtocol> old_protocol) {
+  // If the tracker is ReadyForDestruction(), then it should not be a member of any protocol list.
+  if (tracker->ReadyForDestruction()) {
+    // Since it is not part of any protocol list, it should not have a back pointer to one.
+    DCHECK(!tracker->back_pointer_.has_value());
+    return;
+  }
+
+  if (old_protocol.has_value()) {
+    // If an old protocol is specified, then the tracker should also have been set up
+    // with a back pointer to its list.
+    DCHECK(tracker->back_pointer_.has_value());
+
+    if (old_protocol.value() == tracker->traffic_class().protocol) {
+      // Didn't really move, so nothing to update.
+      return;
+    }
+
+    // Remove tracker from previous list.
+    conn_trackers_by_protocol_[old_protocol.value()].erase(tracker->back_pointer_.value());
+    --num_trackers_in_lists_;
+  } else {
+    // If no old protocol is specified, the the tracker should not be in any list.
+    // Currently, this should only be possible on initialization of a new tracker.
+    DCHECK(!tracker->back_pointer_.has_value());
+  }
+
+  // Add tracker to new list based on its current protocol.
   conn_trackers_by_protocol_[tracker->traffic_class().protocol].push_back(tracker);
+  tracker->back_pointer_ = --conn_trackers_by_protocol_[tracker->traffic_class().protocol].end();
   ++num_trackers_in_lists_;
-  ++num_tracker_dups_;
-  // If this tracker was already part of another protocol list,
-  // we'll remove the tracker from the original protocols list during TransferStreams().
+
+  DebugChecks();
 }
 
 ConnectionTracker& ConnTrackersManager::GetOrCreateConnTracker(struct conn_id_t conn_id) {
   const uint64_t conn_map_key = GetConnMapKey(conn_id.upid.pid, conn_id.fd);
-  DCHECK(conn_map_key != 0) << "Connection map key cannot be 0, pid must be wrong";
+  DCHECK_NE(conn_map_key, 0) << "Connection map key cannot be 0, pid must be wrong";
 
   auto& conn_trackers = connection_trackers_[conn_map_key];
   ConnectionTracker& conn_tracker = conn_trackers[conn_id.tsid];
@@ -36,10 +64,10 @@ ConnectionTracker& ConnTrackersManager::GetOrCreateConnTracker(struct conn_id_t
   // TODO(oazizi): Find a more direct way of detecting new trackers.
   const bool new_tracker = (conn_tracker.conn_id().tsid == 0);
   if (new_tracker) {
-    conn_trackers_by_protocol_[kProtocolUnknown].push_back(&conn_tracker);
-
     ++num_trackers_;
-    ++num_trackers_in_lists_;
+
+    conn_tracker.manager_ = this;
+    UpdateProtocol(&conn_tracker, {});
 
     // If there is a another generation for this conn map key,
     // one of them needs to be marked for death.
@@ -60,6 +88,7 @@ ConnectionTracker& ConnTrackersManager::GetOrCreateConnTracker(struct conn_id_t
     }
   }
 
+  DebugChecks();
   return conn_tracker;
 }
 
@@ -120,33 +149,75 @@ void ConnTrackersManager::CleanupTrackers() {
     }
   }
 
-  DCHECK_EQ(num_trackers_ready_for_destruction_, 0);
+  DebugChecks();
 }
 
-void ConnTrackersManager::CheckConsistency() {
-  DCHECK_EQ(num_trackers_,
-            num_trackers_in_lists_ + num_trackers_ready_for_destruction_ - num_tracker_dups_);
-}
+Status ConnTrackersManager::TestOnlyCheckConsistency() const {
+  // A set used for looking for duplicate trackers.
+  std::set<ConnectionTracker*> trackers_set;
 
-void ConnTrackersManager::DebugInfo() const {
-  int count = 0;
   for (const auto& [protocol, conn_trackers_list] : conn_trackers_by_protocol_) {
-    LOG(INFO) << absl::Substitute("protocol=$0 num_trackers=$1", protocol,
-                                  conn_trackers_list.size());
+    for (auto iter = conn_trackers_list.begin(); iter != conn_trackers_list.end(); ++iter) {
+      ConnectionTracker* tracker = *iter;
 
-    count += conn_trackers_list.size();
+      // Check that tracker exists in connection_trackers_ (i.e. that the pointer is valid).
+      // If the pointer is not valid, this will likely cause a crash or ASAN error.
+      const uint64_t conn_map_key =
+          GetConnMapKey(tracker->conn_id().upid.pid, tracker->conn_id().fd);
+      DCHECK_NE(conn_map_key, 0) << "Connection map key cannot be 0, pid must be wrong";
+      auto tracker_set_it = connection_trackers_.find(conn_map_key);
+      if (tracker_set_it == connection_trackers_.end()) {
+        return error::Internal(
+            "Tracker $0 in the protocol lists not found in connection_trackers_.",
+            ToString(tracker->conn_id()));
+      }
+
+      const auto& tracker_generations = tracker_set_it->second;
+      auto tracker_it = tracker_generations.find(tracker->conn_id().tsid);
+      if (tracker_it == tracker_generations.end()) {
+        return error::Internal(
+            "Tracker $0 in the protocol lists not found in connection_trackers_.",
+            ToString(tracker->conn_id()));
+      }
+
+      // Check that the pointer only shows up once across all lists.
+      // NOLINTNEXTLINE: whitespace/braces
+      auto [unused, inserted] = trackers_set.insert(tracker);
+      if (!inserted) {
+        return error::Internal("Tracker $0 found in two lists.", ToString(tracker->conn_id()));
+      }
+    }
+  }
+
+  return Status::OK();
+}
+
+void ConnTrackersManager::DebugChecks() const {
+  DCHECK_EQ(num_trackers_, num_trackers_in_lists_ + num_trackers_ready_for_destruction_);
+}
+
+std::string ConnTrackersManager::DebugInfo() const {
+  std::string out;
+
+  for (const auto& [protocol, conn_trackers_list] : conn_trackers_by_protocol_) {
+    absl::StrAppend(&out,
+                    absl::Substitute("protocol=$0 num_trackers=$1", magic_enum::enum_name(protocol),
+                                     conn_trackers_list.size()));
 
     for (auto iter = conn_trackers_list.begin(); iter != conn_trackers_list.end(); ++iter) {
       ConnectionTracker* tracker = *iter;
 
-      LOG(INFO) << absl::Substitute("Connection conn_id=$0 protocol=$1 state=$2\n",
-                                    ToString(tracker->conn_id()),
-                                    magic_enum::enum_name(tracker->traffic_class().protocol),
-                                    magic_enum::enum_name(tracker->state()));
+      absl::StrAppend(&out,
+                      absl::Substitute(
+                          "   conn_id=$0 protocol=$1 state=$2 zombie=$3 ready_for_destruction=$4\n",
+                          ToString(tracker->conn_id()),
+                          magic_enum::enum_name(tracker->traffic_class().protocol),
+                          magic_enum::enum_name(tracker->state()), tracker->IsZombie(),
+                          tracker->ReadyForDestruction()));
     }
   }
 
-  LOG(INFO) << absl::Substitute("num_trackers=$0 num_trackers_allocated=$1", count, num_trackers_);
+  return out;
 }
 
 //-----------------------------------------------------------------------------
@@ -155,17 +226,13 @@ void ConnTrackersManager::DebugInfo() const {
 
 ConnTrackersManager::TrackersList::TrackersListIterator::TrackersListIterator(
     std::list<ConnectionTracker*>* trackers, std::list<ConnectionTracker*>::iterator iter,
-    TrafficProtocol protocol, ConnTrackersManager* conn_trackers_manager)
-    : trackers_(trackers),
-      iter_(iter),
-      protocol_(protocol),
-      conn_trackers_manager_(conn_trackers_manager) {
-  AdvanceToValidTracker();
-}
+    ConnTrackersManager* conn_trackers_manager)
+    : trackers_(trackers), iter_(iter), conn_trackers_manager_(conn_trackers_manager) {}
 
 ConnTrackersManager::TrackersList::TrackersListIterator
 ConnTrackersManager::TrackersList::TrackersListIterator::operator++() {
   if ((*iter_)->ReadyForDestruction()) {
+    (*iter_)->back_pointer_.reset();
     trackers_->erase(iter_++);
     --conn_trackers_manager_->num_trackers_in_lists_;
     ++conn_trackers_manager_->num_trackers_ready_for_destruction_;
@@ -173,23 +240,8 @@ ConnTrackersManager::TrackersList::TrackersListIterator::operator++() {
     ++iter_;
   }
 
-  conn_trackers_manager_->CheckConsistency();
-
-  AdvanceToValidTracker();
-
   return *this;
 }
 
-void ConnTrackersManager::TrackersList::TrackersListIterator::AdvanceToValidTracker() {
-  while ((iter_ != trackers_->end()) && ((*iter_)->traffic_class().protocol != protocol_)) {
-    DCHECK_EQ(protocol_, kProtocolUnknown);
-    trackers_->erase(iter_++);
-    --conn_trackers_manager_->num_trackers_in_lists_;
-    --conn_trackers_manager_->num_tracker_dups_;
-  }
-
-  conn_trackers_manager_->CheckConsistency();
-}
-
 }  // namespace stirling
 }  // namespace pl

src/stirling/source_connectors/socket_tracer/conn_trackers_manager.h

@@ -4,6 +4,7 @@
 #include <map>
 #include <memory>
 #include <set>
+#include <string>
 
 #include "src/stirling/source_connectors/socket_tracer/connection_tracker.h"
 
@@ -58,34 +59,27 @@ class ConnTrackersManager {
 
      private:
       TrackersListIterator(std::list<ConnectionTracker*>* trackers,
-                           std::list<ConnectionTracker*>::iterator iter, TrafficProtocol protocol,
+                           std::list<ConnectionTracker*>::iterator iter,
                            ConnTrackersManager* conn_trackers_manager);
 
-      void AdvanceToValidTracker();
-
       std::list<ConnectionTracker*>* trackers_;
       std::list<ConnectionTracker*>::iterator iter_;
-      TrafficProtocol protocol_;
       ConnTrackersManager* conn_trackers_manager_;
 
       friend class TrackersList;
     };
 
     TrackersListIterator begin() {
-      return TrackersListIterator(list_, list_->begin(), protocol_, conn_trackers_);
+      return TrackersListIterator(list_, list_->begin(), conn_trackers_);
     }
 
-    TrackersListIterator end() {
-      return TrackersListIterator(list_, list_->end(), protocol_, conn_trackers_);
-    }
+    TrackersListIterator end() { return TrackersListIterator(list_, list_->end(), conn_trackers_); }
 
    private:
-    TrackersList(std::list<ConnectionTracker*>* list, TrafficProtocol protocol,
-                 ConnTrackersManager* conn_trackers)
-        : list_(list), protocol_(protocol), conn_trackers_(conn_trackers) {}
+    TrackersList(std::list<ConnectionTracker*>* list, ConnTrackersManager* conn_trackers)
+        : list_(list), conn_trackers_(conn_trackers) {}
 
     std::list<ConnectionTracker*>* list_;
-    TrafficProtocol protocol_;
     ConnTrackersManager* conn_trackers_;
 
     friend class ConnTrackersManager;
@@ -95,7 +89,7 @@ class ConnTrackersManager {
    * Returns a list of all the trackers that belong to a particular protocol.
    */
   TrackersList ConnTrackersForProtocol(TrafficProtocol protocol) {
-    return TrackersList(&conn_trackers_by_protocol_[protocol], protocol, this);
+    return TrackersList(&conn_trackers_by_protocol_[protocol], this);
   }
 
   /**
@@ -108,7 +102,7 @@ class ConnTrackersManager {
    * If a connection tracker has its protocol changed, then one must manually call this function.
    * TODO(oazizi): Find a cleaner/more automatic way that can avoid this call altogether.
    */
-  void NotifyProtocolChange(ConnectionTracker* tracker);
+  void UpdateProtocol(ConnectionTracker* tracker, std::optional<TrafficProtocol> old_protocol);
 
   /**
    * Deletes trackers that are ReadyForDestruction().
@@ -118,15 +112,22 @@ class ConnTrackersManager {
   void CleanupTrackers();
 
   /**
-   * Runs checks for checking the consistency of the data structure.
-   * Useful for catching bugs.
+   * Checks the consistency of the data structures.
+   * Useful for catching bugs. Meant for use in testing.
+   * Could be expensive if called too regularly in production.
+   * See DebugChecks() for simpler checks that can be used in production.
+   */
+  Status TestOnlyCheckConsistency() const;
+
+  /**
+   * Simple consistency DCHECKs meant for enforcing invariants.
    */
-  void CheckConsistency();
+  void DebugChecks() const;
 
   /**
-   * Debug utility to dump information about connection trackers.
+   * Returns extensive debug information about the connection trackers.
    */
-  void DebugInfo() const;
+  std::string DebugInfo() const;
 
  private:
   // A map from conn_id (PID+FD+TSID) to tracker. This is for easy update on BPF events.
@@ -148,7 +149,6 @@ class ConnTrackersManager {
   size_t num_trackers_ = 0;
   size_t num_trackers_ready_for_destruction_ = 0;
   size_t num_trackers_in_lists_ = 0;
-  size_t num_tracker_dups_ = 0;
 };
 
 }  // namespace stirling

src/stirling/source_connectors/socket_tracer/conn_trackers_manager_test.cc

+#include <random>
+
+#include "src/common/testing/testing.h"
+#include "src/stirling/source_connectors/socket_tracer/conn_trackers_manager.h"
+
+namespace pl {
+namespace stirling {
+
+class ConnTrackersManagerTest : public ::testing::Test {
+ protected:
+  ConnTrackersManagerTest() : rng_(37), probability_dist_(0.0, 1.0) {}
+
+  ConnTrackersManager trackers_;
+
+  std::default_random_engine rng_;
+  std::uniform_real_distribution<double> probability_dist_;
+
+  void CleanupTrackers() {
+    VLOG(1) << "CleanupTrackers";
+    trackers_.CleanupTrackers();
+  }
+
+  void TransferStreamsProxy(TrafficProtocol protocol, double mark_for_death_probabilty,
+                            int death_countdown) {
+    VLOG(1) << absl::Substitute("TransferStreamsProxy $0 $1", magic_enum::enum_name(protocol),
+                                mark_for_death_probabilty);
+    ConnTrackersManager::TrackersList conn_trackers_list =
+        trackers_.ConnTrackersForProtocol(protocol);
+
+    for (auto iter = conn_trackers_list.begin(); iter != conn_trackers_list.end(); ++iter) {
+      ConnectionTracker* tracker = *iter;
+      if (probability_dist_(rng_) < mark_for_death_probabilty) {
+        tracker->MarkForDeath(death_countdown);
+      }
+    }
+  }
+
+  void TrackerEvent(struct conn_id_t conn_id, TrafficProtocol protocol) {
+    VLOG(1) << "TrackerEvent";
+    ConnectionTracker& tracker = trackers_.GetOrCreateConnTracker(conn_id);
+    tracker.SetConnID(conn_id);
+    tracker.SetProtocol(protocol);
+  }
+};
+
+// This is a stress on ConnTrackersManager.
+// Each iteration, a different action is taken, and the consistency of the structure is checked.
+// ASAN runs can also identify issues while being stressed.
+TEST_F(ConnTrackersManagerTest, Fuzz) {
+  constexpr int kIters = 1000000;
+  std::uniform_int_distribution<int> death_countdown_dist(0, 5);
+  std::uniform_int_distribution<int> pid_dist(1, 5);
+  std::uniform_int_distribution<int> tsid_dist(1, 2);
+  std::uniform_int_distribution<int> protocol_dist(0, kNumProtocols - 1);
+
+  for (int i = 0; i < kIters; ++i) {
+    auto protocol = magic_enum::enum_cast<TrafficProtocol>(protocol_dist(rng_));
+    CHECK(protocol.has_value());
+
+    // Randomly pick an action to take.
+    //  1) TrackerEvent: proxy of a single BPF event from PollPerfBuffers that could change the
+    //  ConnectionTracker state (including protocol). 2) TransferStreamsProxy: proxy of
+    //  TransferStreams(), which processes all the ConnectionTrackers for a given protocol.
+    //      - This can result in a tracker getting into the ReadyForDestruction state.
+    //  3) CleanupTrackers: proxy of CleanupTrackers(), which runs periodically in Stirling.
+    // This is a proxy of what happens in a real Stirling implementation.
+    double x = probability_dist_(rng_);
+    if (x < 0.80) {
+      uint32_t pid = pid_dist(rng_);
+      uint32_t fd = 1;
+      uint64_t tsid = tsid_dist(rng_);
+
+      struct conn_id_t conn_id = {{{pid}, 0}, fd, tsid};
+      TrackerEvent(conn_id, protocol.value());
+    } else if (x < 0.95) {
+      int death_countdown = death_countdown_dist(rng_);
+      double mark_for_death_prob = probability_dist_(rng_);
+      TransferStreamsProxy(protocol.value(), mark_for_death_prob, death_countdown);
+    } else {
+      CleanupTrackers();
+    }
+
+    ASSERT_OK(trackers_.TestOnlyCheckConsistency());
+  }
+}
+
+// This test case is inspired from an elusive bug that caused memory corruption issues due to
+// heap-use-after-free. It was caught by the Fuzz test and boiled down to a simple sequence here.
+// Now that we don't allow a tracker to be in multiple lists, it is less likely to trigger.
+TEST_F(ConnTrackersManagerTest, ChangeProtocolsWhileReadyForDestruction) {
+  struct conn_id_t conn_id = {{{5}, 0}, 1, 12345};
+  constexpr int kDeathCountdown = 0;
+  constexpr double kMarkForDeathProb = 1.0;
+
+  LOG(INFO) << "Add a new event into unknown protocol list.";
+  TrackerEvent(conn_id, kProtocolUnknown);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after adding new event with kProtocolUnknown.";
+
+  LOG(INFO) << "Make tracker ReadyForDestruction by processing the unknown protocols list.";
+  TransferStreamsProxy(kProtocolUnknown, kMarkForDeathProb, kDeathCountdown);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after TransferStreams on kProtocolUnknown.";
+
+  LOG(INFO) << "Process HTTP protocols list. This should have no effect.";
+  TransferStreamsProxy(kProtocolHTTP, kMarkForDeathProb, kDeathCountdown);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after TransferStreams on kProtocolHTTP.";
+
+  LOG(INFO) << "A new event moves the tracker to the HTTP list.";
+  TrackerEvent(conn_id, kProtocolHTTP);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after updating tracker to kProtocolHTTP.";
+
+  LOG(INFO) << "Process unknown protocols list.";
+  TransferStreamsProxy(kProtocolUnknown, kMarkForDeathProb, kDeathCountdown);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after TransferStreams on kProtocolUnknown.";
+
+  LOG(INFO) << "CleanupTrackers. The tracker is removed, so it better not be in any lists.";
+  CleanupTrackers();
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency()) << "Inconsistent state after CleanupTrackers.";
+
+  LOG(INFO) << "Process HTTP protocols list.";
+  TransferStreamsProxy(kProtocolHTTP, kMarkForDeathProb, kDeathCountdown);
+  LOG(INFO) << trackers_.DebugInfo();
+  ASSERT_OK(trackers_.TestOnlyCheckConsistency())
+      << "Inconsistent state after TransferStreams on kProtocolHTTP.";
+}
+
+}  // namespace stirling
+}  // namespace pl

src/stirling/source_connectors/socket_tracer/connection_tracker.cc

@@ -17,6 +17,7 @@
 #include "src/common/system/socket_info.h"
 #include "src/common/system/system.h"
 #include "src/stirling/source_connectors/socket_tracer/bcc_bpf_intf/go_grpc_types.hpp"
+#include "src/stirling/source_connectors/socket_tracer/conn_trackers_manager.h"
 #include "src/stirling/source_connectors/socket_tracer/connection_stats.h"
 #include "src/stirling/source_connectors/socket_tracer/protocols/cql/stitcher.h"
 #include "src/stirling/source_connectors/socket_tracer/protocols/cql/types.h"
@@ -521,7 +522,11 @@ bool ConnectionTracker::SetProtocol(TrafficProtocol protocol) {
     return false;
   }
 
+  TrafficProtocol old_protocol = traffic_class_.protocol;
   traffic_class_.protocol = protocol;
+  if (manager_ != nullptr) {
+    manager_->UpdateProtocol(this, old_protocol);
+  }
   return true;
 }
 
@@ -564,6 +569,8 @@ DataStream* ConnectionTracker::resp_data() {
 }
 
 void ConnectionTracker::MarkForDeath(int32_t countdown) {
+  DCHECK_GE(countdown, 0);
+
   // Only send the first time MarkForDeath is called (death_countdown == -1).
   if (death_countdown_ == -1 && ShouldExportToConnStats()) {
     conn_stats_->AddConnCloseEvent(*this);

src/stirling/source_connectors/socket_tracer/connection_tracker.h

@@ -2,6 +2,7 @@
 
 #include <any>
 #include <deque>
+#include <list>
 #include <map>
 #include <memory>
 #include <string>
@@ -19,6 +20,7 @@
 #include "src/stirling/source_connectors/socket_tracer/fd_resolver.h"
 #include "src/stirling/source_connectors/socket_tracer/protocols/common/interface.h"
 #include "src/stirling/source_connectors/socket_tracer/socket_trace_bpf_tables.h"
+
 // Include all specializations of the StitchFrames() template specializations for all protocols.
 #include "src/stirling/source_connectors/socket_tracer/protocols/stitchers.h"
 
@@ -37,6 +39,7 @@ namespace stirling {
 
 // Forward declaration to avoid circular include of connection_stats.h and connection_tracker.h.
 class ConnectionStats;
+class ConnTrackersManager;
 
 /**
  * @brief Describes a connection from user space. This corresponds to struct conn_info_t in
@@ -624,6 +627,16 @@ class ConnectionTracker : NotCopyMoveable {
 
   template <typename TProtocolTraits>
   friend std::string DebugString(const ConnectionTracker& c, std::string_view prefix);
+
+  // A back pointer to the list in ConnTrackersManager where this tracker is stored.
+  // This member variable is managed completely by ConnTrackersManager.
+  std::optional<std::list<ConnectionTracker*>::iterator> back_pointer_;
+
+  // A pointer to the conn trackers manager, used for notifying a protocol change.
+  ConnTrackersManager* manager_ = nullptr;
+
+  friend class ConnTrackersManager;
+  friend class ConnTrackersManagerTest;
 };
 
 // Explicit template specialization must be declared in namespace scope.

src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc

@@ -907,12 +907,7 @@ void SocketTraceConnector::AcceptDataEvent(std::unique_ptr<SocketDataEvent> even
   ConnectionTracker& tracker = conn_trackers_.GetOrCreateConnTracker(event->attr.conn_id);
   tracker.set_conn_stats(&connection_stats_);
 
-  TrafficProtocol orig_protocol = tracker.traffic_class().protocol;
   tracker.AddDataEvent(std::move(event));
-
-  if (tracker.traffic_class().protocol != orig_protocol) {
-    conn_trackers_.NotifyProtocolChange(&tracker);
-  }
 }
 
 void SocketTraceConnector::AcceptControlEvent(socket_control_event_t event) {
@@ -923,12 +918,7 @@ void SocketTraceConnector::AcceptControlEvent(socket_control_event_t event) {
   ConnectionTracker& tracker = conn_trackers_.GetOrCreateConnTracker(event.open.conn_id);
   tracker.set_conn_stats(&connection_stats_);
 
-  TrafficProtocol orig_protocol = tracker.traffic_class().protocol;
   tracker.AddControlEvent(event);
-
-  if (tracker.traffic_class().protocol != orig_protocol) {
-    conn_trackers_.NotifyProtocolChange(&tracker);
-  }
 }
 
 void SocketTraceConnector::AcceptHTTP2Header(std::unique_ptr<HTTP2HeaderEvent> event) {
@@ -937,12 +927,7 @@ void SocketTraceConnector::AcceptHTTP2Header(std::unique_ptr<HTTP2HeaderEvent> e
   ConnectionTracker& tracker = conn_trackers_.GetOrCreateConnTracker(event->attr.conn_id);
   tracker.set_conn_stats(&connection_stats_);
 
-  TrafficProtocol orig_protocol = tracker.traffic_class().protocol;
   tracker.AddHTTP2Header(std::move(event));
-
-  if (tracker.traffic_class().protocol != orig_protocol) {
-    conn_trackers_.NotifyProtocolChange(&tracker);
-  }
 }
 
 void SocketTraceConnector::AcceptHTTP2Data(std::unique_ptr<HTTP2DataEvent> event) {
@@ -951,12 +936,7 @@ void SocketTraceConnector::AcceptHTTP2Data(std::unique_ptr<HTTP2DataEvent> event
   ConnectionTracker& tracker = conn_trackers_.GetOrCreateConnTracker(event->attr.conn_id);
   tracker.set_conn_stats(&connection_stats_);
 
-  TrafficProtocol orig_protocol = tracker.traffic_class().protocol;
   tracker.AddHTTP2Data(std::move(event));
-
-  if (tracker.traffic_class().protocol != orig_protocol) {
-    conn_trackers_.NotifyProtocolChange(&tracker);
-  }
 }
 
 //-----------------------------------------------------------------------------