i fixed a use-after-free by refcounting on orphaned justifications

Core/SoarKernel/src/decision_process/decide.cpp

@@ -3089,8 +3089,15 @@ void assert_new_preferences(agent* thisAgent, preference_list& bufdeallo)
         {
             if (inst->in_ms)
             {
-                inst->in_newly_created = false;
-                insert_at_head_of_dll(inst->prod->instantiations, inst, next, prev);
+                if (inst->prod)
+                {
+                    inst->in_newly_created = false;
+                    insert_at_head_of_dll(inst->prod->instantiations, inst, next, prev);
+                }
+                else
+                {
+                    inst->in_ms = false;
+                }
             }
 
             if (thisAgent->trace_settings[TRACE_ASSERTIONS_SYSPARAM])

Core/SoarKernel/src/explanation_based_chunking/ebc_build.cpp

@@ -953,7 +953,7 @@ void Explanation_Based_Chunker::learn_rule_from_instance(instantiation* inst, in
     m_chunk_inst->prod_name                         = m_prod->name;
     thisAgent->symbolManager->symbol_add_ref(m_chunk_inst->prod_name);
     m_chunk_inst->in_ms                             = true;                     /* set true for now, we'll find out later... */
-    m_chunk_inst->in_newly_created                  = true;
+    m_chunk_inst->in_newly_created                  = false;
     m_chunk_inst->tested_local_negation             = m_tested_local_negation;
     m_chunk_inst->creates_deep_copy                 = m_tested_deep_copy;
     m_chunk_inst->tested_LTM                        = m_tested_ltm_recall;
@@ -971,6 +971,7 @@ void Explanation_Based_Chunker::learn_rule_from_instance(instantiation* inst, in
         /* --- Add chunk instantiation to list of newly generated instantiations --- */
         m_chunk_inst->next = (*new_inst_list);
         (*new_inst_list) = m_chunk_inst;
+        m_chunk_inst->in_newly_created = true;  /* Set flag AFTER adding to list */
 
         /* Clean up.  (Now that m_chunk_inst s on the list of insts to be asserted, we
          *             set it to to null because so that clean_up() won't delete it.) */

Core/SoarKernel/src/soar_representation/instantiation.cpp

@@ -730,7 +730,7 @@ void finalize_instantiation(agent* thisAgent, instantiation* inst, bool need_to_
 
     /* We don't add a prod refcount for justifications so that they will be
      * excised when they no longer match or no longer have preferences asserted */
-    if (inst->prod && (inst->prod->type != JUSTIFICATION_PRODUCTION_TYPE))
+    if (inst->prod)
     {
         production_add_ref(inst->prod);
     }
@@ -1379,15 +1379,21 @@ void deallocate_instantiation(agent* thisAgent, instantiation*& inst)
 
         if (lDelInst->prod)
         {
-            if ((lDelInst->prod->type == JUSTIFICATION_PRODUCTION_TYPE) && (lDelInst->prod->reference_count == 1))
+            if (lDelInst->prod->type == JUSTIFICATION_PRODUCTION_TYPE)
             {
-                /* We are about to remove a justification that has not been excised from the rete.
-                 * Normally, justifications are excised as soon as they don't have any matches in
-                 * rete.cpp.  But if removing the preference will remove the instantiation, we
-                 * need to excise it now so that the rete doesn't try to later */
-                excise_production(thisAgent, lDelInst->prod, false, true);
-            } else if (lDelInst->prod->type == JUSTIFICATION_PRODUCTION_TYPE) {
-                production_remove_ref(thisAgent, lDelInst->prod);
+                /* If the reference count is 2, it means that the only references are the
+                 * one from the creation of the production and the one from this instantiation.
+                 * We should excise the production now.
+                 * We also check p_node to make sure it hasn't already been excised (e.g. by rete). */
+                if ((lDelInst->prod->reference_count == 2) && (lDelInst->prod->p_node != NIL))
+                {
+                    excise_production(thisAgent, lDelInst->prod, false, true);
+                    production_remove_ref(thisAgent, lDelInst->prod);
+                }
+                else
+                {
+                    production_remove_ref(thisAgent, lDelInst->prod);
+                }
             }
         }
 

Core/SoarKernel/src/soar_representation/production.cpp

@@ -465,6 +465,14 @@ void deallocate_production(agent* thisAgent, production* prod)
         }
     }
 
+    for (instantiation* lInst = thisAgent->newly_created_instantiations; lInst != NULL; lInst = lInst->next)
+    {
+        if (lInst->prod == prod)
+        {
+            lInst->prod = NULL;
+        }
+    }
+
     deallocate_action_list(thisAgent, prod->action_list);
     thisAgent->symbolManager->deallocate_symbol_list_removing_references(prod->rhs_unbound_variables);
     thisAgent->symbolManager->symbol_remove_ref(&prod->name);