Fix access point cleanup on instance removal

src/odb/src/db/dbInst.cpp

@@ -1501,16 +1501,7 @@ void dbInst::destroy(dbInst* inst_)
     _dbITerm* _iterm = block->iterm_tbl_->getPtr(id);
     dbITerm* iterm = (dbITerm*) _iterm;
     iterm->disconnect();
-    if (inst_->getPinAccessIdx() >= 0) {
-      for (const auto& [pin, aps] : iterm->getAccessPoints()) {
-        for (auto ap : aps) {
-          _dbAccessPoint* _ap = (_dbAccessPoint*) ap;
-          auto [first, last] = std::ranges::remove_if(
-              _ap->iterms_, [id](const auto& id_in) { return id_in == id; });
-          _ap->iterms_.erase(first, last);
-        }
-      }
-    }
+    iterm->clearPrefAccessPoints();
 
     // Notify when pins are deleted (assumption: pins are destroyed only when
     // the related instance is destroyed)

src/odb/src/db/dbMPin.cpp

@@ -3,6 +3,8 @@
 
 #include "dbMPin.h"
 
+#include <algorithm>
+#include <cstddef>
 #include <cstdint>
 #include <vector>
 
@@ -16,6 +18,7 @@
 #include "dbMaster.h"
 #include "dbPolygonItr.h"
 #include "dbTable.h"
+#include "dbVector.h"
 #include "odb/db.h"
 #include "odb/dbSet.h"
 #include "odb/geom.h"
@@ -150,11 +153,16 @@ void dbMPin::clearPinAccess(const int pin_access_idx)
 {
   _dbMPin* pin = (_dbMPin*) this;
   _dbBlock* block = (_dbBlock*) getDb()->getChip()->getBlock();
-  if (pin->aps_.size() <= pin_access_idx) {
+  if (pin_access_idx < 0
+      || pin->aps_.size() <= static_cast<std::size_t>(pin_access_idx)) {
     return;
   }
-  const auto aps = pin->aps_[pin_access_idx];
-  for (const auto& ap : aps) {
+  dbVector<dbId<_dbAccessPoint>> aps;
+  aps.swap(pin->aps_[pin_access_idx]);
+  std::ranges::sort(aps);
+  const auto duplicate_aps = std::ranges::unique(aps);
+  aps.erase(duplicate_aps.begin(), duplicate_aps.end());
+  for (const dbId<_dbAccessPoint>& ap : aps) {
     odb::dbAccessPoint::destroy(
         (odb::dbAccessPoint*) block->ap_tbl_->getPtr(ap));
   }

src/odb/test/cpp/BUILD

@@ -30,6 +30,7 @@ cc_test(
     srcs = [
         "TestAccessPoint.cpp",
     ],
+    features = ["-layering_check"],
     deps = [
         "//src/odb",
         "//src/odb/test/cpp/helper",

src/odb/test/cpp/TestAccessPoint.cpp

@@ -4,6 +4,8 @@
 #include <string>
 #include <vector>
 
+#include "../../src/db/dbAccessPoint.h"
+#include "../../src/db/dbMPin.h"
 #include "gtest/gtest.h"
 #include "helper.h"
 #include "odb/db.h"
@@ -76,5 +78,63 @@ TEST_F(SimpleDbFixture, test_default)
   dbDatabase::destroy(db2);
 }
 
+TEST_F(SimpleDbFixture, clear_mpin_access_points_is_idempotent)
+{
+  createSimpleDB();
+  dbBlock* block = db_->getChip()->getBlock();
+  dbMaster* master = db_->findMaster("and2");
+  dbMTerm* term = master->findMTerm("a");
+  dbMPin* pin = dbMPin::create(term);
+  dbAccessPoint* ap = dbAccessPoint::create(block, pin, 0);
+  dbInst* inst = dbInst::create(block, master, "i1");
+  dbITerm* iterm = inst->getITerm(term);
+  iterm->setAccessPoint(pin, ap);
+
+  master->clearPinAccess(0);
+  master->clearPinAccess(0);
+
+  EXPECT_TRUE(iterm->getPrefAccessPoints().empty());
+}
+
+TEST_F(SimpleDbFixture, clear_mpin_access_points_removes_duplicate_references)
+{
+  createSimpleDB();
+  dbBlock* block = db_->getChip()->getBlock();
+  dbMaster* master = db_->findMaster("and2");
+  dbMTerm* term = master->findMTerm("a");
+  dbMPin* pin = dbMPin::create(term);
+  dbAccessPoint* ap = dbAccessPoint::create(block, pin, 0);
+  _dbMPin* pin_impl = static_cast<_dbMPin*>(pin->getImpl());
+
+  // Reproduce duplicate AP bookkeeping that can be left by router pin access
+  // updates before a master pin-access slot is cleared.
+  pin_impl->aps_[0].push_back(ap->getImpl()->getOID());
+  ASSERT_EQ(pin_impl->aps_[0].size(), 2);
+
+  master->clearPinAccess(0);
+
+  EXPECT_TRUE(pin_impl->aps_[0].empty());
+}
+
+TEST_F(SimpleDbFixture, destroy_inst_removes_access_point_back_references)
+{
+  createSimpleDB();
+  dbBlock* block = db_->getChip()->getBlock();
+  dbMaster* master = db_->findMaster("and2");
+  dbMTerm* term = master->findMTerm("a");
+  dbMPin* pin = dbMPin::create(term);
+  dbAccessPoint* ap = dbAccessPoint::create(block, pin, 1);
+  dbInst* inst = dbInst::create(block, master, "i1");
+  dbITerm* iterm = inst->getITerm(term);
+  iterm->setAccessPoint(pin, ap);
+  inst->setPinAccessIdx(0);
+  _dbAccessPoint* ap_impl = static_cast<_dbAccessPoint*>(ap->getImpl());
+  ASSERT_EQ(ap_impl->iterms_.size(), 1);
+
+  dbInst::destroy(inst);
+
+  EXPECT_TRUE(ap_impl->iterms_.empty());
+}
+
 }  // namespace
 }  // namespace odb