Skip to content

Commit 8ce8338

Browse files
johnml1135johnml1135
authored
LT-22388: Fix NullReferenceException in UnitOfWorkService.SaveOnIdle (#361)
Add an early IsDisposed check to prevent accessing UI state (LastActivityTime) when the service has been disposed but the timer event still fires. Cover IsDisposed and null UI paths in SaveOnIdle Filter ICU DLL paths for deterministic CustomIcuFallbackTests Add initial AGENTS onboarding notes and update workspace settings
1 parent f9a164a commit 8ce8338

5 files changed

Lines changed: 207 additions & 4 deletions

File tree

‎.vscode/settings.json‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
{
2-
"csharp.unitTestDebuggingOptions": {
3-
// It would be preferable to "fix" the test projects to target an executable .net core framework (https://stackoverflow.com/a/48885500/2301416)
4-
"type": "clr" // https://github.com/OmniSharp/omnisharp-vscode/wiki/Desktop-.NET-Framework#settingsjson-example
2+
"dotnet.unitTestDebuggingOptions": {
3+
"type": "clr"
54
}
65
}

‎AGENTS.md‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# AGENTS: liblcm (LCM)
2+
3+
## Summary
4+
liblcm (LCM) is the core FieldWorks Language & Culture Model library for linguistic analyses. It provides the data model, serialization, utilities, and tooling for linguistic, anthropological, and text corpus data. It is a multi-project .NET solution with code generation steps and multi-targeting for legacy .NET Framework and modern .NET.
5+
6+
## High-level repo facts
7+
- Type: .NET solution (multi-project class libraries + build tasks + tools + tests).
8+
- Languages: C# (.cs), MSBuild (.proj/.csproj/.props/.targets), XML, shell/batch scripts.
9+
- Target frameworks: net462, netstandard2.0, net8.0 (see .csproj files in src/ and tests/).
10+
- Build tools: MSBuild, dotnet SDK, GitVersion.MsBuild, NUnit.
11+
- Output: artifacts/ (NuGet packages and binaries by configuration/TFM).
12+
13+
## Build and validation (validated commands and observations)
14+
15+
### What CI runs (GitHub Actions)
16+
CI runs on Windows and Ubuntu. See .github/workflows/ci-cd.yml:
17+
1) Install .NET SDK 8.x.
18+
2) Ubuntu: install mono-devel and icu-fw packages.
19+
3) Windows: remove c:\tools\php\icuuc*.dll; install .NET Framework 4.6.1 targeting pack.
20+
4) Build: dotnet build --configuration Release
21+
5) Test:
22+
- Linux: . environ && dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release
23+
- Windows: dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release
24+
6) Pack: dotnet pack --include-symbols --no-restore --no-build -p:SymbolPackageFormat=snupkg --configuration Release
25+
26+
Always mirror this sequence when validating a change locally.
27+
28+
### Local build scripts (not validated here)
29+
- Windows: build.cmd [Debug|Release] [Target] (uses MSBuild on LCM.sln).
30+
- Linux: build.sh [Debug|Release] [Target] (sources environ, uses msbuild on LCM.sln).
31+
These scripts call build/LCM.proj targets (Build/Test/Pack). If you use them, always run from repo root.
32+
33+
### Tests per README (not validated here)
34+
- Windows, ReSharper: open LCM.sln and “Run Unit Tests”.
35+
- Windows, no ReSharper: use MSBuild, then run nunit3-console.exe from artifacts/Debug/net462.
36+
- Linux terminal: source environ, then run mono with nunit3-console.exe on *Tests.dll in artifacts/Debug/net462.
37+
38+
### Commands actually run during onboarding
39+
- dotnet test .\LCM.sln → FAILED
40+
- dotnet build --configuration Release → FAILED
41+
Failure signature (both commands): GitVersion.MsBuild (netcoreapp3.1 gitversion.dll) exited with code 1. This blocks build/test in this environment. CI uses fetch-depth 0, so ensure a full git history is available. If GitVersion still fails, check GitVersion prerequisites and local .NET runtime compatibility.
42+
43+
No command timeouts were observed.
44+
45+
### Known prerequisites and gotchas
46+
- GitVersion.MsBuild is used across projects; it requires git metadata. CI checks out with fetch-depth 0.
47+
- net462 builds on Windows require the .NET Framework 4.6.1 targeting pack (CI installs it).
48+
- ICU data generation requires ICU binaries (CI installs icu-fw on Ubuntu).
49+
- Some projects warn on NU1701; treat as warnings unless build breaks.
50+
- The build prohibits references to System.Windows.Forms (CheckWinForms target).
51+
52+
## Project layout and architecture
53+
54+
### Key solution and build files
55+
- LCM.sln: solution entry point.
56+
- build.cmd / build.sh: wrapper scripts for MSBuild.
57+
- build/LCM.proj: orchestrated build/test/pack, uses NUnit console on output/ for legacy builds.
58+
- Directory.Build.props / Directory.Build.targets: repo-wide build settings and packaging.
59+
- Directory.Solution.props / Directory.Solution.targets: solution-level defaults.
60+
- GitVersion.yml: GitVersion configuration.
61+
- global.json: SDK roll-forward config.
62+
- .editorconfig: formatting rules.
63+
64+
### Major source projects (src/)
65+
- src/SIL.LCModel: main LCM library (net462; netstandard2.0).
66+
- src/SIL.LCModel.Core: core utilities and ICU data generation (netstandard2.0; net462; net8.0).
67+
- src/SIL.LCModel.Utils: shared utilities (net462; netstandard2.0).
68+
- src/SIL.LCModel.Build.Tasks: MSBuild tasks used for code generation.
69+
- src/SIL.LCModel.FixData: data-fix utilities.
70+
- src/CSTools: auxiliary tools (pg/lg/Tools).
71+
72+
Code generation targets to know about:
73+
- SIL.LCModel: GenerateModel (MasterLCModel.xml → Generated*.cs).
74+
- SIL.LCModel.Core: GenerateKernelCs, GenerateIcuData.
75+
76+
### Tests (tests/)
77+
- SIL.LCModel.Tests
78+
- SIL.LCModel.Core.Tests
79+
- SIL.LCModel.Utils.Tests
80+
- SIL.LCModel.FixData.Tests
81+
- TestHelper (support project)
82+
83+
### CI/validation checks
84+
- GitHub Actions: .github/workflows/ci-cd.yml (build, test, pack, publish).
85+
- Tests run with dotnet test and ParallelizeAssembly=false.
86+
- Packaging uses dotnet pack with symbol packages.
87+
88+
### Dependencies not obvious from layout
89+
- ICU data and binaries (icu-fw) for Core ICU generation.
90+
- Mono on Linux for some runtime/test workflows.
91+
- GitVersion.MsBuild for versioning (requires git metadata).
92+
93+
## Root files list
94+
- .editorconfig
95+
- .gitattributes
96+
- .gitignore
97+
- build.cmd
98+
- build.sh
99+
- CHANGELOG.md
100+
- Directory.Build.props
101+
- Directory.Build.targets
102+
- Directory.Solution.props
103+
- Directory.Solution.targets
104+
- environ
105+
- GitVersion.yml
106+
- global.json
107+
- LCM.sln
108+
- LCM.sln.DotSettings
109+
- LICENSE
110+
- README.md
111+
112+
## Repo top-level directories
113+
- .github/ (GitHub Actions workflow)
114+
- .vscode/ (VS settings)
115+
- artifacts/ (build outputs)
116+
- build/ (LCM.proj)
117+
- src/ (production code)
118+
- tests/ (unit tests)
119+
120+
## README highlights (summary)
121+
- Describes liblcm as FieldWorks model library for linguistic analyses.
122+
- Build: use build.cmd (Windows) or build.sh (Linux). Default Debug, optional Release.
123+
- Debugging: use LOCAL_NUGET_REPO to publish local packages; see NuGet local feeds.
124+
- Tests: Windows via ReSharper or NUnit console; Linux via mono + NUnit console (requires environ).
125+
126+
## Trust these instructions
127+
Follow this file first. Only search the repo if these instructions are incomplete or prove incorrect for your task.

‎src/SIL.LCModel/Infrastructure/Impl/UnitOfWorkService.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,12 @@ void SaveOnIdle(object sender, ElapsedEventArgs e)
230230
// Check if we are already in SaveInternal.
231231
if (m_fInSaveInternal)
232232
return;
233+
// Check if we are disposed before accessing m_ui
234+
if (IsDisposed)
235+
return;
236+
if (m_ui == null)
237+
return;
238+
233239
// Don't save if we're in the middle of something and not in the right state to Save!
234240
if (UndoOrRedoInProgress || CurrentProcessingState != BusinessTransactionState.ReadyForBeginTask)
235241
return; // don't start another, if for example the conflict dialog is open.

‎tests/SIL.LCModel.Core.Tests/Text/CustomIcuFallbackTests.cs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ public void TearDown()
164164
public void FixtureSetUp()
165165
{
166166
// Undo the PATH that got set by the InitializeIcu attribute
167-
Environment.SetEnvironmentVariable("PATH", InitializeIcuAttribute.PreTestPathEnvironment);
167+
var originalPath = InitializeIcuAttribute.PreTestPathEnvironment;
168+
Environment.SetEnvironmentVariable("PATH", RemoveIcuPaths(originalPath));
168169
_dirsToDelete = new List<string>();
169170
_preTestDataDir = Wrapper.DataDirectory;
170171
_preTestDataDirEnv = Environment.GetEnvironmentVariable("ICU_DATA");
@@ -237,6 +238,31 @@ private static void PrintIcuDllsOnPath()
237238
}
238239
}
239240

241+
private static string RemoveIcuPaths(string path)
242+
{
243+
if (string.IsNullOrEmpty(path))
244+
return path;
245+
246+
var filtered = new List<string>();
247+
foreach (var folder in path.Split(Path.PathSeparator))
248+
{
249+
if (string.IsNullOrWhiteSpace(folder))
250+
continue;
251+
try
252+
{
253+
if (Directory.Exists(folder) && Directory.EnumerateFiles(folder, "icuuc*.dll").Any())
254+
continue;
255+
}
256+
catch
257+
{
258+
// If we can't enumerate the folder, keep it to avoid breaking PATH unexpectedly.
259+
}
260+
filtered.Add(folder);
261+
}
262+
263+
return string.Join(Path.PathSeparator.ToString(), filtered);
264+
}
265+
240266
[Test]
241267
public void InitIcuDataDir_NoIcuLibrary()
242268
{

‎tests/SIL.LCModel.Tests/Infrastructure/Impl/UnitOfWorkServiceTests.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright (c) 2026 SIL International
2+
// This software is licensed under the LGPL, version 2.1 or later
3+
// (http://www.gnu.org/licenses/lgpl-2.1.html)
4+
5+
using System;
6+
using System.Reflection;
7+
using NUnit.Framework;
8+
using SIL.LCModel;
9+
using SIL.LCModel.Core.KernelInterfaces;
10+
11+
namespace SIL.LCModel.Infrastructure.Impl
12+
{
13+
[TestFixture]
14+
public class UnitOfWorkServiceTests : MemoryOnlyBackendProviderTestBase
15+
{
16+
[Test]
17+
public void SaveOnIdle_UiCleared_DoesNotThrow()
18+
{
19+
var uowService = Cache.ServiceLocator.GetInstance<IUnitOfWorkService>();
20+
var serviceInstance = (object)uowService;
21+
22+
InvokeNonPublicVoid(serviceInstance, "StopSaveTimer");
23+
SetNonPublicField(serviceInstance, "m_ui", null);
24+
25+
Assert.DoesNotThrow(() =>
26+
InvokeNonPublicVoid(serviceInstance, "SaveOnIdle", null, null));
27+
}
28+
29+
private static void SetNonPublicField(object instance, string fieldName, object value)
30+
{
31+
var field = instance.GetType().GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic);
32+
if (field == null)
33+
Assert.Fail("Field not found: " + fieldName);
34+
field.SetValue(instance, value);
35+
}
36+
37+
private static void InvokeNonPublicVoid(object instance, string methodName, params object[] args)
38+
{
39+
var method = instance.GetType().GetMethod(methodName, BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);
40+
if (method == null)
41+
Assert.Fail("Method not found: " + methodName);
42+
method.Invoke(instance, args);
43+
}
44+
}
45+
}

0 commit comments

Comments
 (0)