DeepWiki-found bugs

https://deepwiki.com/sillsdev/liblcm
https://deepwiki.com/search/find-bugs_563f89be-f25d-46f1-9d57-0afdf13b424c?mode=deep

Here are the bugs found across the codebase, ordered roughly by severity:

Bug 1: NullReferenceException in MoMorphData.ActiveParser setter

File: src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs

				XmlDocument doc = new XmlDocument();
				XmlNode paramsNode = null;
				try
				{
					doc.LoadXml(ParserParameters);
					paramsNode = doc.SelectSingleNode("/ParserParameters");
				}
				catch
				{
					// Eat exception.
				}

				if (paramsNode == null)
				{
					paramsNode = doc.CreateElement("ParserParameters");
					doc.DocumentElement.AppendChild(paramsNode);
				}

When doc.LoadXml(ParserParameters) throws (e.g., ParserParameters is null or malformed XML), the exception is silently swallowed and paramsNode stays null. The code then calls doc.DocumentElement.AppendChild(paramsNode), but doc.DocumentElement is null on a freshly constructed XmlDocument, causing a NullReferenceException. The fix is doc.AppendChild(paramsNode).

Bug 2: Skipped run after removal in CleanChapterInBtPara

File: src/SIL.LCModel/DomainImpl/ScrTxtPara.cs

		private void CleanChapterInBtPara(int wsAlt, BCVRef startRef, BCVRef endRef)
		{
			int iRun = 0;

			while (iRun < GetOrCreateBT().Translation.get_String(wsAlt).RunCount)
			{
				RemoveOutOfRangeChapterRun(iRun, wsAlt, startRef, endRef);
				iRun++;
			}
		}

iRun is unconditionally incremented even when RemoveOutOfRangeChapterRun returns true (meaning a run was deleted and all subsequent runs shifted down by one). The run that slides into position iRun is never examined. Two consecutive out-of-range chapter runs will cause the second one to be silently skipped.

Bug 3: Potential NullReferenceException in MergeSelectedPropertiesOfObject

File: src/SIL.LCModel/DomainImpl/CmObject.cs

							var myAddMethod = myCurrentValue.GetType().GetMethod("Add");
							foreach (var input in ((ILcmVector)srcCurrentValue).Objects)
								myAddMethod.Invoke(myCurrentValue, new object[] { input });

myCurrentValue.GetType().GetMethod("Add") can return null if the runtime type doesn't expose a public Add method with the expected signature (e.g., due to explicit interface implementation). The result is used immediately without a null check, so myAddMethod.Invoke(...) will throw a NullReferenceException.

Bug 4: Silent data loss in CollectVars for sequence contexts

File: src/SIL.LCModel/DomainImpl/OverridesLing_MoClasses.cs

				case PhSequenceContextTags.kClassId:
					var seqCtxt = (IPhSequenceContext)ctxt;
					foreach (var cur in seqCtxt.MembersRS)
						CollectVars(cur as IPhSimpleContextNC, featureConstrs);
					break;

In the PhSequenceContextTags.kClassId case, each member is cast with cur as IPhSimpleContextNC. If a member is not an IPhSimpleContextNC (e.g., it is a PhIterationContext or PhSequenceContext), the cast silently returns null, and the null-guard at the top of CollectVars returns early — those members' variables are never collected.

Bug 5: Managed object access from finalizer in UnitOfWorkService.Dispose

File: src/SIL.LCModel/Infrastructure/Impl/UnitOfWorkService.cs

			if (fDisposing && !IsDisposed)
			{
				// dispose managed and unmanaged objects
				m_lock.Dispose();
				m_saveTimer.Dispose();
			}
			IsDisposed = true;
			m_logger?.AddBreadCrumb("UOW Disposed.");
		}

m_logger?.AddBreadCrumb("UOW Disposed.") is placed outside the if (fDisposing && !IsDisposed) guard. When called from the finalizer (fDisposing == false), this accesses the managed m_logger object, which may already have been garbage-collected. This violates the standard dispose pattern and can cause unpredictable behavior.

Bug 6: NullReferenceException in WsString.Equals and GetHashCode

File: src/SIL.LCModel/Application/ApplicationServices/XmlImportData.cs

			public override bool Equals(object obj)
			{
				if (obj is WsString)
				{
					return (ws == ((WsString)obj).ws) && sVal.Equals(((WsString)obj).sVal);
				}
				else
				{
					return false;
				}
			}

			public override int GetHashCode()
			{
				return sVal.GetHashCode() + (ws % 100);
			}

WsString.Equals calls sVal.Equals(...) and GetHashCode calls sVal.GetHashCode() without guarding against sVal being null. If a WsString is constructed with a null string value, both methods will throw NullReferenceException.

Bug 7: Wrong exception type in IndexOfFirstUnusedId

File: src/SIL.LCModel/DomainServices/ITextUtils.cs

		static protected int IndexOfFirstUnusedId(List<int> ids)
		{
			if (ids == null)
				throw new NullReferenceException("ids");
			return IndexOfFirstUnusedId(ids.ToArray());
		}

The method explicitly throws new NullReferenceException("ids") for a null argument. This should be ArgumentNullException, which is the standard .NET convention for null parameter validation. NullReferenceException is reserved for unintentional null dereferences.

Bug 8: Potential deadlock in FileTransactionLogger finalizer

File: src/SIL.LCModel/FileTransactionLogger.cs

	   protected virtual void Dispose(bool disposing)
		{
			Debug.WriteLineIf(!disposing, "****** Missing Dispose() call for " + GetType() + ". *******");
			lock (m_lock)
			{
				if (!m_disposed)
				{
					try { m_lastWrite?.GetAwaiter().GetResult(); }
					catch { /* best-effort: don't let a failed write prevent disposal */ }
					m_stream?.Flush();
					m_stream?.Dispose();
					m_stream = null;
					m_disposed = true;
				}
			}
		}

The finalizer calls Dispose(false), which acquires m_lock and then calls m_lastWrite?.GetAwaiter().GetResult(). Blocking on a Task from a finalizer thread is dangerous: if the continuation was scheduled to run on the finalizer thread (via ExecuteSynchronously), it cannot run while the finalizer thread is blocked waiting for it — a deadlock. The finalizer path should skip the GetResult() call entirely.

Review the eight reported entry points, especially OverridesLing_MoClasses.cs, ScrTxtPara.cs, CmObject.cs, UnitOfWorkService.cs, XmlImportData.cs, ITextUtils.cs, and FileTransactionLogger.cs. Validate each finding against its surrounding code and existing tests; done means the selected defects are confirmed, corrected, and covered by appropriate regression tests.