excise o-supported justifications after one fire

Also lots of debug printing.

This doesn't fix the T-S agent, unfortunately :( agent loops forever on go-to.

Core/SoarKernel/src/decision_process/decide.cpp

@@ -3164,6 +3164,7 @@ void assert_new_preferences(agent* thisAgent, preference_list& bufdeallo)
             lDeleteInst = *iter;
             lDeleteInst->in_newly_created = false;
             lDeleteInst->in_newly_deleted = false;
+            thisAgent->outputManager->printa_sf(thisAgent, "Call deallocate_instantiation from assert_new_preferences for: %y\n", lDeleteInst->prod_name);
             deallocate_instantiation(thisAgent, lDeleteInst);
         }
         thisAgent->newly_deleted_instantiations.clear();

Core/SoarKernel/src/decision_process/rete.cpp

@@ -3883,6 +3883,9 @@ byte add_production_to_rete(agent* thisAgent, production* p, condition* lhs_top,
                               msc, next_in_level, prev_in_level);
 
 #ifdef BUG_139_WORKAROUND
+    if (msc->p_node->b.p.prod->type == JUSTIFICATION_PRODUCTION_TYPE) {
+        thisAgent->outputManager->printa_sf(thisAgent, "Setting already_fired to 0 for %y\n", msc->p_node->b.p.prod->name);
+    }
         msc->p_node->b.p.prod->already_fired = 0;       /* RPM workaround for bug #139; mark prod as not fired yet */
 #endif
 
@@ -6128,7 +6131,7 @@ void p_node_left_addition(agent* thisAgent, rete_node* node, token* tok, wme* w)
             i need to talk to him about it. */
 
         }  /* end if (operator_proposal == false) */
-        thisAgent->outputManager->printa_sf(thisAgent, "rete: Production %y is %s-supported\n", node->b.p.prod->name, prod_type);
+        thisAgent->outputManager->printa_sf(thisAgent, "rete: Production %y is %d-supported\n", node->b.p.prod->name, prod_type);
     }        /* end UNDECLARED_SUPPORT */
 
     if (prod_type == PE_PRODS)

Core/SoarKernel/src/soar_representation/instantiation.cpp

@@ -1208,6 +1208,7 @@ void create_instantiation(agent* thisAgent, production* prod, struct token_struc
 
 void deallocate_instantiation(agent* thisAgent, instantiation*& inst)
 {
+    thisAgent->outputManager->printa_sf(thisAgent,  "Deallocating instantiation %y.\n", inst->prod_name);
     if (inst->in_newly_created)
     {
         if (!inst->in_newly_deleted)
@@ -1387,7 +1388,11 @@ void deallocate_instantiation(agent* thisAgent, instantiation*& inst)
                  * 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 {
+            }
+            else {
+                if (lDelInst->prod->type == JUSTIFICATION_PRODUCTION_TYPE) {
+                    thisAgent->outputManager->printa_sf(thisAgent,  "Justification %y has reference count %d, so won't excise\n", lDelInst->prod_name, lDelInst->prod->reference_count);
+                }
                 production_remove_ref(thisAgent, lDelInst->prod);
             }
         }
@@ -1420,9 +1425,16 @@ void retract_instantiation(agent* thisAgent, instantiation* inst)
     /* retract any preferences that are in TM and aren't o-supported */
     pref = inst->preferences_generated;
 
+    bool o_supported = false;
     while (pref != NIL)
     {
+        if (pref->o_supported)
+        {
+            o_supported = true;
+        }
         next = pref->inst_next;
+        // causes infinite success/put-in loop
+        // if (pref->in_tm && (!pref->o_supported || (inst->prod->type == JUSTIFICATION_PRODUCTION_TYPE)))
         if (pref->in_tm && (!pref->o_supported))
         {
 
@@ -1453,11 +1465,11 @@ void retract_instantiation(agent* thisAgent, instantiation* inst)
     /* prod may not exist if rule was manually excised */
     if (inst->prod)
     {
-        /* remove inst from list of instantiations of this production */
-        remove_from_dll(inst->prod->instantiations, inst, next, prev);
-
         production* prod = inst->prod;
 
+        /* remove inst from list of instantiations of this production */
+        remove_from_dll(prod->instantiations, inst, next, prev);
+
         if (prod->type == CHUNK_PRODUCTION_TYPE)
         {
             rl_param_container::apoptosis_choices apoptosis = thisAgent->RL->rl_params->apoptosis->get_value();
@@ -1480,7 +1492,17 @@ void retract_instantiation(agent* thisAgent, instantiation* inst)
 
     /* mark as no longer in MS, and possibly deallocate  */
     inst->in_ms = false;
+    thisAgent->outputManager->printa_sf(thisAgent, "Calling possibly_deallocate_instantiation from retract_instantiation: %y.\n", inst->prod_name);
+    thisAgent->outputManager->printa_sf(thisAgent, "possibly_deallocate_instantiation, not in_ms: %d\n", (!(inst)->in_ms));
+    thisAgent->outputManager->printa_sf(thisAgent, "possibly_deallocate_instantiation, no preferences_generated: %d\n", (!(inst)->preferences_generated));
     possibly_deallocate_instantiation(thisAgent, inst);
+    if (inst->prod && inst->prod->type == JUSTIFICATION_PRODUCTION_TYPE && o_supported && (inst->prod->already_fired)) {
+        // Justifications only have one instantiation, and this one is o-supported so it can't ever re-assert itself.
+        // We therefore excise the production to prevent possible issues with duplicate justifications later.
+        // see https://github.com/SoarGroup/Soar/issues/529.
+        thisAgent->outputManager->printa_sf(thisAgent,  "Justification %y is o-supported and already fired, so excising\n", inst->prod_name);
+        excise_production(thisAgent, inst->prod, false, true);
+    }
 }
 
 instantiation* make_architectural_instantiation_for_memory_system(agent* thisAgent, Symbol* pState, wme_set* pConds, symbol_triple_list* pActions, bool forSMem)

Core/SoarKernel/src/soar_representation/preference.cpp

@@ -24,7 +24,7 @@
    memory, however.
 -------------------------------------------------------------------*/
 
-preference* make_preference(agent* thisAgent, PreferenceType type, 
+preference* make_preference(agent* thisAgent, PreferenceType type,
                             Symbol* id, Symbol* attr, Symbol* value, Symbol* referent,
                             const identity_quadruple &o_ids, const bool_quadruple &pWas_unbound_vars)
 {
@@ -197,6 +197,7 @@ void deallocate_preference(agent* thisAgent, preference* pref, bool dont_cache)
     {
         if (!dont_cache) cache_preference_if_necessary(thisAgent, pref);
         remove_from_dll(pref->inst->preferences_generated, pref, inst_next, inst_prev);
+        thisAgent->outputManager->printa_sf(thisAgent,  "Calling possibly_deallocate_instantiation from deallocate_preference: %y ^%y %y\n", pref->id, pref->attr, pref->value);
         possibly_deallocate_instantiation(thisAgent, pref->inst);
     }
 

Core/SoarKernel/src/soar_representation/production.cpp

@@ -454,6 +454,7 @@ production* make_production(agent*          thisAgent,
 void deallocate_production(agent* thisAgent, production* prod)
 {
     if (!prod) return;
+    thisAgent->outputManager->printa_sf(thisAgent,  "deallocating production %y\n", prod->name);
     if (prod->instantiations)
     {
         /* Soar used to abort here, but I think this can easily happen with the
@@ -479,6 +480,7 @@ void deallocate_production(agent* thisAgent, production* prod)
 
 void excise_production(agent* thisAgent, production* prod, bool print_sharp_sign, bool cacheProdForExplainer)
 {
+    thisAgent->outputManager->printa_sf(thisAgent,  "Excising production %y\n", prod->name);
     /* When excising, the explainer needs to save the production before we excise it from
      * the RETE.  Otherwise, it won't be able to reconstruct the cached conditions/actions */
     if (cacheProdForExplainer && prod->save_for_justification_explanation && thisAgent->explanationMemory->is_any_enabled())