core: guard null central particle in bond breakage angle branch (bug-sweep #26)

src/core/bond_breakage/actions.hpp

@@ -23,6 +23,8 @@
 
 #include <array>
 #include <cstddef>
+#include <unordered_set>
+#include <variant>
 
 namespace BondBreakage {
 
@@ -91,3 +93,13 @@ template <> struct hash<BondBreakage::DeleteAllBonds> {
   }
 };
 } // namespace std
+
+namespace BondBreakage {
+
+// Variant holding any of the actions
+using Action = std::variant<DeleteBond, DeleteAngleBond, DeleteAllBonds>;
+
+// Set of actions
+using ActionSet = std::unordered_set<Action>;
+
+} // namespace BondBreakage

src/core/bond_breakage/bond_breakage.cpp

@@ -45,12 +45,6 @@
 
 namespace BondBreakage {
 
-// Variant holding any of the actions
-using Action = std::variant<DeleteBond, DeleteAngleBond, DeleteAllBonds>;
-
-// Set of actions
-using ActionSet = std::unordered_set<Action>;
-
 /** Add a particle+bond combination to the breakage queue */
 void BondBreakage::queue_breakage(int particle_id,
                                   BondPartners const &bond_partners,
@@ -72,9 +66,8 @@ static auto gather_global_queue(Queue const &local_queue) {
 }
 
 /** @brief Constructs the actions to take for a breakage queue entry */
-static ActionSet actions_for_breakage(CellStructure const &cell_structure,
-                                      QueueEntry const &e,
-                                      BreakageSpec const &spec) {
+ActionSet actions_for_breakage(CellStructure const &cell_structure,
+                               QueueEntry const &e, BreakageSpec const &spec) {
   auto is_angle_bond = [](auto const &bond_partners) {
     return bond_partners[1];
   }; // optional for second partner engaged
@@ -121,7 +114,12 @@ static ActionSet actions_for_breakage(CellStructure const &cell_structure,
     auto vs = cell_structure.get_local_particle(e.particle_id);
     auto p1 = cell_structure.get_local_particle(*(e.bond_partners[0]));
     auto p2 = cell_structure.get_local_particle(*(e.bond_partners[1]));
-    if (p1 and p2) {
+    // The global breakage queue is broadcast to every rank, but only the rank
+    // that holds the central virtual site builds the action; on a rank where
+    // the central particle is absent (legitimate multi-rank locality), return
+    // an empty action set instead of dereferencing a null pointer. This mirrors
+    // the central-particle guard of the pair branch above.
+    if (vs and p1 and p2) {
       if (not vs->is_virtual()) {
         runtimeErrorMsg() << "The REVERT_BIND_AT_POINT_OF_COLLISION bond "
                              "breakage action has to be configured for the "

src/core/bond_breakage/bond_breakage.hpp

@@ -21,6 +21,7 @@
 
 #include <config/config.hpp>
 
+#include "bond_breakage/actions.hpp"
 #include "system/System.hpp"
 
 #include <boost/serialization/access.hpp>
@@ -32,6 +33,8 @@
 #include <unordered_map>
 #include <vector>
 
+class CellStructure;
+
 namespace BondBreakage {
 
 /** Stores one or two bond partners for pair/angle bonds */
@@ -67,6 +70,17 @@ struct QueueEntry {
 /** @brief Record bonds broken during a time step. */
 using Queue = std::vector<QueueEntry>;
 
+/**
+ * @brief Construct the set of delete actions for a single breakage queue entry.
+ *
+ * Each MPI rank receives the broadcast global queue and only builds actions
+ * for the particles it actually holds locally; referenced particles that are
+ * not resolvable on this rank (@ref CellStructure::get_local_particle returns
+ * @c nullptr) yield an empty action set. Exposed (non-static) for unit testing.
+ */
+ActionSet actions_for_breakage(CellStructure const &cell_structure,
+                               QueueEntry const &e, BreakageSpec const &spec);
+
 class BondBreakage {
   Queue m_queue;
 

src/core/unit_tests/bond_breakage_test.cpp

@@ -21,7 +21,28 @@
 #define BOOST_TEST_DYN_LINK
 #include <boost/test/unit_test.hpp>
 
+#include "config/config.hpp"
+
+#include "EspressoCoreGlobalConfig.hpp"
+
 #include "bond_breakage/actions.hpp"
+#include "bond_breakage/bond_breakage.hpp"
+
+#include "Particle.hpp"
+#include "PropagationMode.hpp"
+#include "cell_system/CellStructure.hpp"
+#include "cell_system/CellStructureType.hpp"
+#include "system/System.hpp"
+
+#include <memory>
+#include <optional>
+#include <utility>
+
+struct GlobalConfig : public EspressoCoreGlobalConfig {
+  ~GlobalConfig() { ::System::reset_system(); }
+};
+
+BOOST_TEST_GLOBAL_CONFIGURATION(GlobalConfig);
 
 BOOST_AUTO_TEST_CASE(test_actions_equality) {
   {
@@ -76,3 +97,63 @@ BOOST_AUTO_TEST_CASE(test_actions_hash_value) {
     BOOST_CHECK((Action{1, 2}.hash_value() != Action{1, 0}.hash_value()));
   }
 }
+
+#ifdef ESPRESSO_VIRTUAL_SITES_RELATIVE
+// Add a virtual particle with the given id to the cell structure.
+static void add_virtual_particle(CellStructure &cell_structure, int id) {
+  Particle p;
+  p.id() = id;
+  p.propagation() = PropagationMode::TRANS_VS_RELATIVE;
+  p.vs_relative().to_particle_id = id + 100;
+  cell_structure.add_particle(std::move(p));
+}
+
+/**
+ * Regression test for the unguarded null dereference of the central particle
+ * (@c vs) in the REVERT_BIND_AT_POINT_OF_COLLISION angle-bond branch of
+ * @ref BondBreakage::actions_for_breakage (bug-sweep #26).
+ *
+ * In a multi-rank run the global breakage queue is broadcast to every rank, so
+ * each rank runs @c actions_for_breakage for every entry, including entries
+ * whose central virtual-site particle is not local/ghost on this rank. We
+ * reproduce that locality condition on a single rank by populating a cell
+ * structure with the two partner particles but NOT the central particle:
+ * @c get_local_particle(central_id) returns @c nullptr while the partners
+ * resolve. The buggy code dereferences @c vs->is_virtual() unconditionally
+ * (segfault); the fixed code must return an empty action set.
+ */
+BOOST_AUTO_TEST_CASE(actions_for_breakage_angle_missing_central_particle) {
+  auto system = System::System::create();
+  System::set_system(system);
+  system->set_cell_structure_topology(CellStructureType::NSQUARE);
+  auto &cell_structure = *system->cell_structure;
+
+  auto const central_id = 0; // never added -> not resolvable on this rank
+  auto const partner_id_1 = 1;
+  auto const partner_id_2 = 2;
+
+  // both partners are present, the central virtual site is absent
+  add_virtual_particle(cell_structure, partner_id_1);
+  add_virtual_particle(cell_structure, partner_id_2);
+
+  BOOST_REQUIRE(cell_structure.get_local_particle(central_id) == nullptr);
+  BOOST_REQUIRE(cell_structure.get_local_particle(partner_id_1) != nullptr);
+  BOOST_REQUIRE(cell_structure.get_local_particle(partner_id_2) != nullptr);
+
+  BondBreakage::QueueEntry entry{
+      central_id,
+      {{std::optional<int>{partner_id_1}, std::optional<int>{partner_id_2}}},
+      /* bond_type */ 0};
+  BondBreakage::BreakageSpec spec{
+      /* breakage_length */ 0.,
+      BondBreakage::ActionType::REVERT_BIND_AT_POINT_OF_COLLISION};
+
+  // Must not dereference the null central particle; an absent-on-rank central
+  // particle is a legitimate locality condition, so no action is produced.
+  auto const actions =
+      BondBreakage::actions_for_breakage(cell_structure, entry, spec);
+  BOOST_CHECK_EQUAL(actions.size(), 0u);
+
+  System::reset_system();
+}
+#endif // ESPRESSO_VIRTUAL_SITES_RELATIVE