diff --git a/Core/SoarKernel/src/decision_process/decide.cpp b/Core/SoarKernel/src/decision_process/decide.cpp index d10d7cb7c3..3c7df0cc5a 100644 --- a/Core/SoarKernel/src/decision_process/decide.cpp +++ b/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(); diff --git a/Core/SoarKernel/src/decision_process/rete.cpp b/Core/SoarKernel/src/decision_process/rete.cpp index 16c9a21378..5bb73a60ae 100644 --- a/Core/SoarKernel/src/decision_process/rete.cpp +++ b/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) diff --git a/Core/SoarKernel/src/soar_representation/instantiation.cpp b/Core/SoarKernel/src/soar_representation/instantiation.cpp index b1ff2806ee..1dbadc97e3 100644 --- a/Core/SoarKernel/src/soar_representation/instantiation.cpp +++ b/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) diff --git a/Core/SoarKernel/src/soar_representation/preference.cpp b/Core/SoarKernel/src/soar_representation/preference.cpp index 2776377e17..b9df03bf59 100644 --- a/Core/SoarKernel/src/soar_representation/preference.cpp +++ b/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); } diff --git a/Core/SoarKernel/src/soar_representation/production.cpp b/Core/SoarKernel/src/soar_representation/production.cpp index 22ccf73575..16b5ceb512 100644 --- a/Core/SoarKernel/src/soar_representation/production.cpp +++ b/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())