Make the attack link symmetric, following spring-attachment semantics

core/PhysiCell_cell.cpp

@@ -238,12 +238,15 @@ Cell_State::Cell_State()
 	neighbors.resize(0); 
 	spring_attachments.resize(0); 
 
+	attacked_by.resize(0);
+
 	orientation.resize( 3 , 0.0 ); 
 
 	simple_pressure = 0.0; 
 
 	attached_cells.clear(); 
 	spring_attachments.clear(); 
+	attacked_by.clear();
 
 	number_of_nuclei = 1; 
 
@@ -457,6 +460,8 @@ Cell::~Cell()
 		{
 			// release any attached cells (as of 1.7.2 release)
 			this->remove_all_attached_cells(); 
+			this->remove_all_attackers();
+			this->remove_self_from_attacked();
 			// 1.11.0
 			this->remove_all_spring_attachments(); 
 
@@ -549,8 +554,12 @@ Cell* Cell::divide( )
 	// phenotype.flagged_for_division = false; 
 	// phenotype.flagged_for_removal = false; 
 
-	// make sure ot remove adhesions 
+	// make sure to remove adhesions 
 	remove_all_attached_cells(); 
+	// the springs below include the one holding an attack together, so end the
+	// attack rather than leave it spring-less
+	remove_all_attackers();
+	remove_self_from_attacked();
 	remove_all_spring_attachments(); 
 
 	// version 1.10.3: 
@@ -651,6 +660,7 @@ Cell* Cell::divide( )
 	set_total_volume(phenotype.volume.total);
 
 	// child->set_phenotype( phenotype ); 
+	// pAttackTarget was cleared above, so the daughter inherits a NULL one
 	child->phenotype = phenotype; 
 
     if (child->phenotype.intracellular){
@@ -1144,6 +1154,14 @@ void Cell::convert_to_cell_definition( Cell_Definition& cd )
 	Geometry cell_geometry = phenotype.geometry;
 	Molecular cell_molecular = phenotype.molecular;
 	Custom_Cell_Data cell_custom_data = custom_data;
+
+	// pAttackTarget is phenotype state, and the assignment below replaces it, so
+	// end this cell's own attack. attacked_by and the spring are in state, which
+	// transformation preserves, so attacks against this cell continue.
+	remove_self_from_attacked();
+	// should we also remove all attackers?  That would be a change in behavior, so for now we don't.
+	// remove_all_attackers();
+
 	// use the cell defaults; 
 	type = cd.type; 
 	type_name = cd.name; 
@@ -1210,6 +1228,8 @@ void delete_cell( int index )
 
 	// release any attached cells (as of 1.7.2 release)
 	pDeleteMe->remove_all_attached_cells(); 
+	pDeleteMe->remove_all_attackers();
+	pDeleteMe->remove_self_from_attacked();
 	// 1.11.0 
 	pDeleteMe->remove_all_spring_attachments(); 
 
@@ -1244,6 +1264,8 @@ void delete_cell_original( int index ) // before June 11, 2020
 
 	// release any attached cells (as of 1.7.2 release)
 	(*all_cells)[index]->remove_all_attached_cells(); 
+	(*all_cells)[index]->remove_all_attackers();
+	(*all_cells)[index]->remove_self_from_attacked();
 	// 1.11.0
 	(*all_cells)[index]->remove_all_spring_attachments(); 
 
@@ -1470,10 +1492,13 @@ void Cell::ingest_cell( Cell* pCell_to_eat )
 
 	}
 
-	// things that have their own thread safety 
+	// Teardown is deferred rather than done here. flag_for_removal() queues this
+	// cell, and the serial cells_ready_to_die -> die() -> delete_cell() pass does
+	// the same four steps off-thread. Doing them here means mutating other cells'
+	// lists from inside the mechanics parallel-for, where remove_all_* walks a
+	// vector unlocked while attach/detach_cell_as_spring edits it under the
+	// critical -- a real race, not a theoretical one. 
 	pCell_to_eat->flag_for_removal();
-	pCell_to_eat->remove_all_attached_cells();
-	pCell_to_eat->remove_all_spring_attachments();
 
 	return; 
 }
@@ -1639,9 +1664,13 @@ void Cell::fuse_cell( Cell* pCell_to_fuse )
 	}
 
 	// things that have their own thread safety 
+	// Teardown is deferred rather than done here. flag_for_removal() queues this
+	// cell, and the serial cells_ready_to_die -> die() -> delete_cell() pass does
+	// the same four steps off-thread. Doing them here means mutating other cells'
+	// lists from inside the mechanics parallel-for, where remove_all_* walks a
+	// vector unlocked while attach/detach_cell_as_spring edits it under the
+	// critical -- a real race, not a theoretical one. 
 	pCell_to_fuse->flag_for_removal();
-	pCell_to_fuse->remove_all_attached_cells();
-	pCell_to_fuse->remove_all_spring_attachments();
 
 	return; 
 }
@@ -1652,12 +1681,12 @@ void Cell::lyse_cell( void )
 	if( phenotype.volume.total < 1e-15 )
 	{ return; } 	
 
+	// mark it as dead (before flagging, to match ingest_cell and fuse_cell)
+	phenotype.death.dead = true;
+
 	// flag for removal 
 	flag_for_removal(); // should be safe now 
 
-	// mark it as dead 
-	phenotype.death.dead = true; 
-
 	// set secretion and uptake to zero 
 	phenotype.secretion.set_all_secretion_to_zero( );  
 	phenotype.secretion.set_all_uptake_to_zero( ); 
@@ -1671,7 +1700,12 @@ void Cell::lyse_cell( void )
 
 	// remove all adhesions 
 
-	remove_all_attached_cells(); 
+	// Teardown is deferred rather than done here. flag_for_removal() queues this
+	// cell, and the serial cells_ready_to_die -> die() -> delete_cell() pass does
+	// the same four steps off-thread. Doing them here means mutating other cells'
+	// lists from inside the mechanics parallel-for, where remove_all_* walks a
+	// vector unlocked while attach/detach_cell_as_spring edits it under the
+	// critical -- a real race, not a theoretical one. 
 
 	// set volume to zero 
 	set_total_volume( 0.0 ); 
@@ -3457,6 +3491,78 @@ void Cell::remove_all_spring_attachments( void )
 	return; 
 }
 
+void Cell::add_attacker( Cell* pAddMe )
+{
+	#pragma omp critical
+	{
+		if( std::find( state.attacked_by.begin() , state.attacked_by.end() , pAddMe )
+			== state.attacked_by.end() )
+		{ state.attacked_by.push_back( pAddMe ); }
+	}
+	return;
+}
+
+void Cell::remove_attacker( Cell* pRemoveMe )
+{
+	#pragma omp critical
+	{
+		auto search = std::find( state.attacked_by.begin() , state.attacked_by.end() , pRemoveMe );
+		if( search != state.attacked_by.end() )
+		{
+			// copy last entry over it, then shrink by one
+			*search = state.attacked_by.back();
+			state.attacked_by.pop_back();
+		}
+	}
+	return;
+}
+
+void Cell::remove_all_attackers( void )
+{
+	// Swap the list out under the same lock add_attacker / remove_attacker use, then
+	// walk the local copy. Holding the lock across the loop is not an option:
+	// detach_cells_as_spring() takes the same unnamed critical and OpenMP criticals
+	// are not reentrant, so that would deadlock.
+	std::vector<Cell*> attackers;
+	#pragma omp critical
+	{ attackers.swap( state.attacked_by ); }
+
+	for( int i = 0; i < attackers.size() ; i++ )
+	{
+		Cell* pAttacker = attackers[i];
+		if( pAttacker->phenotype.cell_interactions.pAttackTarget == this )
+		{ pAttacker->phenotype.cell_interactions.pAttackTarget = NULL; }
+		detach_cells_as_spring( pAttacker , this );
+	}
+	return;
+}
+
+bool Cell::is_attacked_by( Cell* pCell )
+{
+	bool found = false;
+	#pragma omp critical
+	{
+		found = std::find( state.attacked_by.begin() , state.attacked_by.end() , pCell )
+			!= state.attacked_by.end();
+	}
+	return found;
+}
+
+void Cell::remove_self_from_attacked( void )
+{
+	Cell* pTarget = phenotype.cell_interactions.pAttackTarget;
+	if( pTarget == NULL )
+	{ return; }
+	phenotype.cell_interactions.pAttackTarget = NULL;
+	pTarget->remove_attacker( this );
+	// mutual attackers share one spring, so only drop it once both are done.
+	// "does pTarget attack me?" is asked of our own attacked_by, which we can lock,
+	// rather than by reading pTarget's pAttackTarget unsynchronised.
+	if( !is_attacked_by( pTarget ) )
+	{ detach_cells_as_spring( this , pTarget ); }
+	return;
+}
+
 
 void attach_cells( Cell* pCell_1, Cell* pCell_2 )
 {
@@ -3483,6 +3589,39 @@ void detach_cells_as_spring( Cell* pCell_1 , Cell* pCell_2 )
 {
 	pCell_1->detach_cell_as_spring( pCell_2 );
 	pCell_2->detach_cell_as_spring( pCell_1 );
+	return;
+}
+
+void begin_attack( Cell* pAttacker , Cell* pTarget )
+{
+	// no self-attack, and nothing to do without both ends
+	if( pAttacker == NULL || pTarget == NULL || pAttacker == pTarget )
+	{ return; }
+
+	// a cell attacks at most one target at a time. if it is already attacking
+	// something, close that attack out cleanly rather than orphaning the link.
+	if( pAttacker->phenotype.cell_interactions.pAttackTarget != NULL )
+	{ pAttacker->remove_self_from_attacked(); }
+
+	pAttacker->phenotype.cell_interactions.pAttackTarget = pTarget;
+	pTarget->add_attacker( pAttacker );
+	// spring-link these cells
+	attach_cells_as_spring( pAttacker , pTarget );
+	return;
+}
+
+void end_attack( Cell* pAttacker , Cell* pTarget )
+{
+	if( pAttacker == NULL || pTarget == NULL )
+	{ return; }
+
+	if( pAttacker->phenotype.cell_interactions.pAttackTarget == pTarget )
+	{ pAttacker->phenotype.cell_interactions.pAttackTarget = NULL; }
+	pTarget->remove_attacker( pAttacker );
+	// two cells can attack each other, and attach_cell_as_spring dedupes, so the
+	// two attacks share ONE spring. Only drop it once nobody still needs it.
+	if( !pAttacker->is_attacked_by( pTarget ) )
+	{ detach_cells_as_spring( pAttacker , pTarget ); }
 	return; 
 }
 

core/PhysiCell_cell.h

@@ -145,6 +145,10 @@ class Cell_State
 	std::vector<Cell*> attached_cells; 
 	std::vector<Cell*> spring_attachments; 
 
+	// reverse of phenotype.cell_interactions.pAttackTarget. in state, not
+	// phenotype, so it survives convert_to_cell_definition()
+	std::vector<Cell*> attacked_by;
+
 	std::vector<Cell*> neighbors; 
 	std::vector<double> orientation;
 
@@ -239,6 +243,13 @@ class Cell : public Basic_Agent
 	void detach_cell_as_spring( Cell* pRemoveMe ); // done 
 	void remove_all_spring_attachments( void ); // done 
 
+	void add_attacker( Cell* pAddMe ); // pAddMe has started attacking me
+	void remove_attacker( Cell* pRemoveMe ); // pRemoveMe has stopped attacking me
+	// call these wherever remove_all_spring_attachments() is called
+	void remove_all_attackers( void ); // everyone attacking me stops
+	void remove_self_from_attacked( void ); // I stop attacking whomever I attack
+	bool is_attacked_by( Cell* pCell ); // is pCell currently attacking me?
+
 	// I want to eventually deprecate this, by ensuring that 
 	// critical BioFVM and PhysiCell data elements are synced when they are needed 
 
@@ -296,6 +307,11 @@ void detach_cells( Cell* pCell_1 , Cell* pCell_2 );
 void attach_cells_as_spring( Cell* pCell_1, Cell* pCell_2 );
 void detach_cells_as_spring( Cell* pCell_1 , Cell* pCell_2 );
 
+// maintain pAttackTarget, attacked_by, and the spring together. use these
+// rather than writing pAttackTarget directly.
+void begin_attack( Cell* pAttacker , Cell* pTarget );
+void end_attack( Cell* pAttacker , Cell* pTarget );
+
 
 std::vector<Cell*> find_nearby_cells( Cell* pCell ); // new in 1.8.0
 std::vector<Cell*> find_nearby_interacting_cells( Cell* pCell ); // new in 1.8.0

core/PhysiCell_standard_models.cpp

@@ -1265,16 +1265,16 @@ void standard_cell_cell_interactions( Cell* pCell, Phenotype& phenotype, double
 			{
 				if( UniformRandom() < probability ) 
 				{				
-					pCell->phenotype.cell_interactions.pAttackTarget = pTarget; 
+					// sets pAttackTarget, registers pCell in pTarget's
+					// attacked_by, and spring-links the two
+					begin_attack( pCell , pTarget );
 					attacked = true; 
 					/*					
 					std::cout << "*********   *********  ********  start atack **** " << PhysiCell_globals.current_time << std::endl; 
 					std::cout 
 					<< "attack duration: " << pCell->phenotype.cell_interactions.attack_duration << " "  
 					<< "attack damage rate: " << pCell->phenotype.cell_interactions.attack_damage_rate <<  std::endl; 
 					*/
-					// spring-link these cells 
-					attach_cells_as_spring(pCell,pTarget); 
 				} 
 			}
 
@@ -1316,11 +1316,14 @@ void standard_cell_cell_interactions( Cell* pCell, Phenotype& phenotype, double
 
 	// move effector attack here. 
 
-		if( pCell->phenotype.cell_interactions.pAttackTarget != NULL ) 
+		// read the target once. attack_cell() dereferences without a null check, so
+		// testing this field and then reloading it would let any concurrent clear
+		// hand it a NULL. One load is free and does not depend on which teardown
+		// paths currently run inside this loop. 
+		Cell* pAttackTarget = pCell->phenotype.cell_interactions.pAttackTarget; 
+		if( pAttackTarget != NULL ) 
 		{
-			Cell* pTarget = pCell->phenotype.cell_interactions.pAttackTarget; 
-
-			pCell->attack_cell(pTarget,dt); 
+			pCell->attack_cell(pAttackTarget,dt); 
 			attacked = true; // attacked at least one cell in this time step 
 
 			// attack_cell
@@ -1330,20 +1333,18 @@ void standard_cell_cell_interactions( Cell* pCell, Phenotype& phenotype, double
 			probability = dt / (1e-15 + pCell->phenotype.cell_interactions.attack_duration); 
 
 
-			if( UniformRandom() < probability || pTarget->phenotype.death.dead ) 
+			if( UniformRandom() < probability || pAttackTarget->phenotype.death.dead ) 
 			{
 				/*
 				std::cout << "*********   *********  ********  attack done **** " << PhysiCell_globals.current_time << " " 
 				<< probability << " "
-				<< "attack time: " << pTarget->state.total_attack_time << " " 		
-				<< "damage: " << pTarget->phenotype.cell_integrity.damage <<  " " 		
-				<< "dead? " << (int) pTarget->phenotype.death.dead << " " 
+				<< "attack time: " << pAttackTarget->state.total_attack_time << " " 		
+				<< "damage: " << pAttackTarget->phenotype.cell_integrity.damage <<  " " 		
+				<< "dead? " << (int) pAttackTarget->phenotype.death.dead << " " 
 				<< "damage delivered: " << pCell->phenotype.cell_interactions.total_damage_delivered << std::endl; 
 				*/
 
-				detach_cells_as_spring(pCell,pTarget); 
-
-				pCell->phenotype.cell_interactions.pAttackTarget = NULL; 
+				end_attack( pCell , pAttackTarget );
 			} 
 		} 
 

modules/PhysiCell_MultiCellDS.cpp

@@ -2196,7 +2196,10 @@ int recreate_sim_state(std::string filename, Microenvironment& M,
             {
                 if (cell->ID == pair.second)
                 {
-                    (pair.first)->phenotype.cell_interactions.pAttackTarget = cell;
+                    // restore all three records, not just the pointer: the spring
+                    // is not serialised, so without this a resumed run starts with
+                    // an attack that has no spring behind it
+                    begin_attack( pair.first , cell );
                     if (debug_print)
                     { std::cout << "    cell ID=" << (pair.first)->ID << " attacking  cell ID=" << cell->ID << std::endl; }
                     break;