diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8b4bb61..d0850fb5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed
+- [SIL.LCModel] Deleting a duplicated phonological rule no longer deletes pooled feature constraints and environment contexts the original rule still references (LT-22575)
- [SIL.LCModel] Fixed crash when bulk deleting entries involved in a lexical relation (LT-21598)
- [SIL.LCModel.FixData] Find and Fix removes duplicate Targets from a LexReference (and deletes it if fewer than two distinct Targets remain) (LT-21598)
- [SIL.LCModel] Data migration now serializes dates using the culture-neutral `ToLCMTimeFormatWithMillisString` (LT-20698)
diff --git a/src/SIL.LCModel/DomainImpl/OverridesLing_Lex.cs b/src/SIL.LCModel/DomainImpl/OverridesLing_Lex.cs
index 7ef2899f..1a57a9e7 100644
--- a/src/SIL.LCModel/DomainImpl/OverridesLing_Lex.cs
+++ b/src/SIL.LCModel/DomainImpl/OverridesLing_Lex.cs
@@ -7635,7 +7635,11 @@ protected override void OnBeforeObjectDeleted()
{
base.OnBeforeObjectDeleted();
foreach (var constr in FeatureConstraints)
- m_cache.LanguageProject.PhonologicalDataOA.FeatConstraintsOS.Remove(constr);
+ {
+ // GC the pooled constraint unless a duplicate rule or a process still references it (LT-22575).
+ if (constr.ReferringObjects.All(o => o is IPhSimpleContextNC ctxt && ctxt.Rule == this))
+ m_cache.LanguageProject.PhonologicalDataOA.FeatConstraintsOS.Remove(constr);
+ }
}
protected override void RemoveObjectSideEffectsInternal(RemoveObjectEventArgs e)
@@ -7646,10 +7650,10 @@ protected override void RemoveObjectSideEffectsInternal(RemoveObjectEventArgs e)
var removedCtxt = e.ObjectRemoved as IPhSimpleContextNC;
if (removedCtxt != null)
{
- var featConstrs = GetFeatureConstraintsExcept(removedCtxt);
foreach (var constr in removedCtxt.PlusConstrRS.ToList())
{
- if (!featConstrs.Contains(constr))
+ // GC the pooled constraint only if removedCtxt is its last referrer (LT-22575).
+ if (constr.ReferringObjects.All(o => o == removedCtxt))
{
// Remove constr from removedCtxt first to avoid deleting removedCtxt twice (fixes LT-21729).
removedCtxt.PlusConstrRS.Remove(constr);
@@ -7658,7 +7662,7 @@ protected override void RemoveObjectSideEffectsInternal(RemoveObjectEventArgs e)
}
foreach (var constr in removedCtxt.MinusConstrRS.ToList())
{
- if (!featConstrs.Contains(constr))
+ if (constr.ReferringObjects.All(o => o == removedCtxt))
{
// Remove constr from removedCtxt first to avoid deleting removedCtxt twice.
removedCtxt.MinusConstrRS.Remove(constr);
@@ -8201,7 +8205,8 @@ protected override void SetDefaultValuesAfterInit()
protected override void OnBeforeObjectDeleted()
{
base.OnBeforeObjectDeleted();
- if (MemberRA != null)
+ // GC the pooled member only if this context is its last referrer (LT-22575).
+ if (MemberRA != null && MemberRA.ReferringObjects.All(o => o == this))
m_cache.LanguageProject.PhonologicalDataOA.ContextsOS.Remove(MemberRA);
}
}
@@ -8212,7 +8217,11 @@ protected override void OnBeforeObjectDeleted()
{
base.OnBeforeObjectDeleted();
foreach (var ctxt in MembersRS.ToArray())
- m_cache.LanguageProject.PhonologicalDataOA.ContextsOS.Remove(ctxt);
+ {
+ // GC the pooled member only if this sequence is its last referrer (LT-22575).
+ if (ctxt.ReferringObjects.All(o => o == this))
+ m_cache.LanguageProject.PhonologicalDataOA.ContextsOS.Remove(ctxt);
+ }
}
protected override void RemoveObjectSideEffectsInternal(RemoveObjectEventArgs e)
diff --git a/tests/SIL.LCModel.Tests/DomainImpl/LingTests.cs b/tests/SIL.LCModel.Tests/DomainImpl/LingTests.cs
index 94964aaf..c5a08872 100644
--- a/tests/SIL.LCModel.Tests/DomainImpl/LingTests.cs
+++ b/tests/SIL.LCModel.Tests/DomainImpl/LingTests.cs
@@ -147,6 +147,149 @@ public void MoAffixProcessFeatureConstraints()
Assert.That(featureConstrs, Has.Count.EqualTo(3));
}
+ ///
+ /// LT-22575: feature constraints are owned by the PhPhonData.FeatConstraintsOS pool and only
+ /// referenced by rule contexts, so CopyObject hands a duplicate references to the original's
+ /// constraints, not copies. The rule's delete-time GC must skip constraints with outside referrers.
+ ///
+ [Test]
+ public void DuplicateRegularRule_DeletingCloneDoesNotDeleteOriginalsFeatureConstraints()
+ {
+ var phData = Cache.LangProject.PhonologicalDataOA;
+
+ // Build a regular rule whose structural description contains a natural-class context that
+ // uses an alpha variable (a PhFeatureConstraint).
+ var rule = Cache.ServiceLocator.GetInstance().Create();
+ phData.PhonRulesOS.Add(rule);
+ var ctxt = Cache.ServiceLocator.GetInstance().Create();
+ rule.StrucDescOS.Add(ctxt);
+ var featConstr = Cache.ServiceLocator.GetInstance().Create();
+ phData.FeatConstraintsOS.Add(featConstr);
+ ctxt.PlusConstrRS.Add(featConstr);
+
+ Assert.That(rule.FeatureConstraints.Count(), Is.EqualTo(1), "original rule should have one feature constraint");
+ Assert.That(phData.FeatConstraintsOS.Count, Is.EqualTo(1));
+
+ // Duplicate the rule the same way the UI does (this drives PhRegularRule.SetCloneProperties).
+ var clone = (IPhRegularRule)CopyObject.CloneLcmObject(rule, x => phData.PhonRulesOS.Add(x));
+ Assert.That(phData.PhonRulesOS.Count, Is.EqualTo(2), "clone should have been created");
+
+ // Delete the clone.
+ Cache.DomainDataByFlid.DeleteObj(clone.Hvo);
+
+ // The original rule must be completely unaffected by deleting its clone.
+ Assert.That(rule.IsValidObject, Is.True, "original rule should still exist");
+ Assert.That(rule.StrucDescOS.Count, Is.EqualTo(1), "original rule should still have its context");
+ Assert.That(rule.FeatureConstraints.Count(), Is.EqualTo(1),
+ "deleting the cloned rule must not remove the original rule's feature constraint");
+ Assert.That(rule.FeatureConstraints.First().IsValidObject, Is.True,
+ "the original rule's feature constraint must still be a valid object");
+ }
+
+ ///
+ /// LT-22575: one alpha variable referenced from both a StrucDesc (plus) and an RHS StrucChange
+ /// (minus) context, shared with the duplicate rather than copied. Deletes the original first to
+ /// prove survival follows remaining referrers, then checks the pool empties (GC still completes).
+ ///
+ [Test]
+ public void DuplicateRegularRule_SharedConstraintSurvivesUntilLastRuleDeleted()
+ {
+ var phData = Cache.LangProject.PhonologicalDataOA;
+
+ var rule = Cache.ServiceLocator.GetInstance().Create();
+ phData.PhonRulesOS.Add(rule);
+
+ // One alpha variable used in two owned contexts: StrucDesc (plus) and RHS StrucChange (minus).
+ var constr = Cache.ServiceLocator.GetInstance().Create();
+ phData.FeatConstraintsOS.Add(constr);
+ var descCtxt = Cache.ServiceLocator.GetInstance().Create();
+ rule.StrucDescOS.Add(descCtxt);
+ descCtxt.PlusConstrRS.Add(constr);
+ var changeCtxt = Cache.ServiceLocator.GetInstance().Create();
+ rule.RightHandSidesOS[0].StrucChangeOS.Add(changeCtxt);
+ changeCtxt.MinusConstrRS.Add(constr);
+
+ var clone = (IPhRegularRule)CopyObject.CloneLcmObject(rule, x => phData.PhonRulesOS.Add(x));
+
+ // The clone references the same pooled constraint; no copies are made.
+ Assert.That(clone.FeatureConstraints.Single(), Is.EqualTo(constr));
+ Assert.That(phData.FeatConstraintsOS.Count, Is.EqualTo(1));
+
+ // Deleting the original must leave the shared constraint for the clone.
+ Cache.DomainDataByFlid.DeleteObj(rule.Hvo);
+ Assert.That(constr.IsValidObject, Is.True, "constraint is still referenced by the clone");
+ Assert.That(clone.FeatureConstraints.Single(), Is.EqualTo(constr));
+
+ // Deleting the last rule that uses it removes it from the pool.
+ Cache.DomainDataByFlid.DeleteObj(clone.Hvo);
+ Assert.That(phData.FeatConstraintsOS.Count, Is.EqualTo(0), "last referrer gone, constraint should be GCed");
+ }
+
+ ///
+ /// LT-22575: covers the edit path (RemoveObjectSideEffectsInternal on StrucDesc removal), which
+ /// previously GCed a constraint unused elsewhere in the same rule even when a duplicate used it.
+ /// The last-referrer check must span all rules, and still GC once no referrer remains.
+ ///
+ [Test]
+ public void DuplicateRegularRule_RemovingContextKeepsConstraintUsedByClone()
+ {
+ var phData = Cache.LangProject.PhonologicalDataOA;
+
+ var rule = Cache.ServiceLocator.GetInstance().Create();
+ phData.PhonRulesOS.Add(rule);
+ var ctxt = Cache.ServiceLocator.GetInstance().Create();
+ rule.StrucDescOS.Add(ctxt);
+ var constr = Cache.ServiceLocator.GetInstance().Create();
+ phData.FeatConstraintsOS.Add(constr);
+ ctxt.PlusConstrRS.Add(constr);
+
+ var clone = (IPhRegularRule)CopyObject.CloneLcmObject(rule, x => phData.PhonRulesOS.Add(x));
+
+ // Editing the original: removing its context must not GC the constraint the clone still uses.
+ rule.StrucDescOS.Remove(ctxt);
+ Assert.That(constr.IsValidObject, Is.True, "constraint is still referenced by the clone");
+ Assert.That(clone.FeatureConstraints.Single(), Is.EqualTo(constr));
+
+ // Removing the clone's context too leaves no referrers, so the constraint is GCed.
+ clone.StrucDescOS.Remove(clone.StrucDescOS[0]);
+ Assert.That(phData.FeatConstraintsOS.Count, Is.EqualTo(0));
+ }
+
+ ///
+ /// LT-22575: sequence-environment members live in PhPhonData.ContextsOS and are held only by
+ /// reference (MembersRS), so both rules share them. Unguarded GC deleted them, emptying the
+ /// original's sequence, which then nulled the RHS LeftContextOA — silently wiping the environment.
+ ///
+ [Test]
+ public void DuplicateRegularRule_DeletingCloneKeepsSharedEnvironmentMembers()
+ {
+ var phData = Cache.LangProject.PhonologicalDataOA;
+
+ var rule = Cache.ServiceLocator.GetInstance().Create();
+ phData.PhonRulesOS.Add(rule);
+ var rhs = rule.RightHandSidesOS[0];
+ var seqCtxt = Cache.ServiceLocator.GetInstance().Create();
+ rhs.LeftContextOA = seqCtxt;
+ var member1 = Cache.ServiceLocator.GetInstance().Create();
+ phData.ContextsOS.Add(member1);
+ seqCtxt.MembersRS.Add(member1);
+ var member2 = Cache.ServiceLocator.GetInstance().Create();
+ phData.ContextsOS.Add(member2);
+ seqCtxt.MembersRS.Add(member2);
+
+ var clone = (IPhRegularRule)CopyObject.CloneLcmObject(rule, x => phData.PhonRulesOS.Add(x));
+
+ // Deleting the duplicate must leave the original's environment intact.
+ Cache.DomainDataByFlid.DeleteObj(clone.Hvo);
+ Assert.That(rhs.LeftContextOA, Is.EqualTo(seqCtxt), "original environment should survive");
+ Assert.That(seqCtxt.MembersRS.Count, Is.EqualTo(2));
+ Assert.That(phData.ContextsOS.Count, Is.EqualTo(2));
+
+ // Deleting the original GCs the pooled members.
+ Cache.DomainDataByFlid.DeleteObj(rule.Hvo);
+ Assert.That(phData.ContextsOS.Count, Is.EqualTo(0));
+ }
+
///
/// Trivial test that a newly created InflAffixTemplate has the Final property set to true.
///