[htsugawa]

Summary

LcmsAnalysisModelWithoutRawTests.ASampleWithoutRawDataOpensFromWhatTheProjectStores (added in #829) fails in Debug: its cleanup cannot delete sample.dcl. There were two causes:

1. The test never disposed the file model. In the application, AnalysisFileBeanModel owns and caches the .dcl reader (MSDecLoader). LcmsAnalysisModel borrows that reader through MsDecSpectrumFromFileLoader, and the dataset's AnalysisFileBeanModelCollection disposes it. The test plays the dataset's part but never disposed the file model. In Release the JIT let the local die early, so the test's own GC.Collect() finalized the FileStream and the test passed by accident. In Debug the local stays alive until the method returns, so the handle was still open when Cleanup ran.
2. A product bug in DisposableCollection. When I made the test dispose the file model, the .dcl still stayed open. While an application runs, DisposableCollection.Dispose queues its items' disposal on the dispatcher (InvokeAsync, Background, since Release msidal5.3 #373). DisposableModelBase.Dispose then calls Disposables.Clear() as soon as that call returns (since Implement analysis pipeline and UI for TIMS-off imaging #639). The queued callback enumerates the collection when it runs, finds it empty, and disposes nothing. So since Implement analysis pipeline and UI for TIMS-off imaging #639 a DisposableModelBase disposed none of its items while MS-DIAL ran. When Dispose was called off the UI thread, the clear could also race the enumeration.

The no-raw browsing path does not leak a handle of its own. I checked this with exclusive-open probes at each step:

step	.dcl open	.pai2 open
inside the LcmsAnalysisModel	yes (the file model's cached reader)	no
after LcmsAnalysisModel.Dispose()	yes (borrowed, so this is correct)	no
after file.ReleaseMSDecLoader()	no	no
after file.Dispose() with an application running, before the fix	yes	no

Changes

• DisposableCollection: copy the items before queuing their disposal. Disposal still happens on the dispatcher at Background priority, as Release msidal5.3 #373 intended, and Clear() still drops the references, as Implement analysis pipeline and UI for TIMS-off imaging #639 intended.
• LcmsAnalysisModelWithoutRawTests:
  • Cleanup disposes the file model and waits for the dispatcher before deleting the folder.
  • The test now disposes the file model, as closing the dataset would, and asserts that neither the .dcl nor the .pai2 is still open. The check runs before any GC.Collect, so a finalizer cannot hide a leak.
• AnalysisFileBeanModelTests.ReleasingTheLoaderOrDisposingTheModelClosesTheDeconvolutionFile (new) runs with an application. It checks that ReleaseMSDecLoader() (used before reprocessing rewrites the .dcl) and Dispose() both close the .dcl.
• Utility/TestApplication and Utility/TestFiles are shared test helpers. TestApplication is the STA application that LcmsAnalysisModelWithoutRawTests used to build privately, plus a wait for Background-priority dispatcher work.

Behaviour change to review

82 source files define classes that derive from DisposableModelBase. Since #639, none of them disposed their Disposables while the GUI ran; they now do, deferred to the dispatcher as before #639. This is the intended behaviour, but an object that is registered with AddTo(Disposables) and still used elsewhere would now be disposed where it previously was not. Such double-ownership bugs could have been introduced since September 2025. I have not exercised the running GUI with this change, only the test suites below.

Tests

Built with dotnet (net48), default NuGet sources, in the worktree MsdialWorkbench-dcl-lock.

• Before the fix: the original test fails alone in Debug (IOException: ... 'sample.dcl' because it is being used by another process) and passes in Release. The new and adjusted tests fail in both Debug and Release on the .dcl still being open after Dispose().
• After the fix: MsdialGuiAppTests passed 145/145 in Debug and in Release, three full runs each before a final helper refactor and one each after it. The target test also passes alone in both. CommonMVVMTests passed 16/16.

🤖 Generated with Claude Code

[htsugawa]

Added 0df2b42 after checking why #832, #834, #835 and #836 fail PR Check.

All four fail on the one test this PR fixes, LcmsAnalysisModelWithoutRawTests.ASampleWithoutRawDataOpensFromWhatTheProjectStores. Their own steps pass (on #834: Check lock files, Restore packages and Build all succeed):

• Lipid RT-order check: assign Confirmed to lipid annotations whose retention times follow the reversed-phase rules #832, Make the Master Build restore pass: regenerate the Release vendor unsupported lock files #834, Keep Model free of ViewModel and WPF types: models ask for messages and the file picker through the broker #836: TestCleanup ... IOException: The process cannot access the file 'sample.dcl', the leak this PR closes.
• Record each raw data file's format at registration, so mzTab-M ms_run-format survives the raw data #835: Assert.IsTrue failed. Opening took 00:00:02.8027823. The < 2 s bound fails a cold runner, although Record each raw data file's format at registration, so mzTab-M ms_run-format survives the raw data #835 adds nothing to the timed open (it never reaches the format classifier when the raw data are missing).

The bound exists to catch the old freeze, where reading the missing raw data slept 5 s before each retry (BaseDataProvider, Thread.Sleep(5000)). It is now < 5 s, which still fails any retry. Passed locally three times.

Once this is merged, the other four branches need master merged in (or a re-run) to pick it up.

🤖 Generated with Claude Code