diff --git a/src/Common/CommonMVVM/Common/DisposableCollection.cs b/src/Common/CommonMVVM/Common/DisposableCollection.cs index 2ac116e62..4134621f0 100644 --- a/src/Common/CommonMVVM/Common/DisposableCollection.cs +++ b/src/Common/CommonMVVM/Common/DisposableCollection.cs @@ -1,5 +1,6 @@ using System; using System.Collections.ObjectModel; +using System.Linq; using System.Windows; namespace CompMs.CommonMVVM.Common @@ -33,9 +34,12 @@ protected virtual void Dispose(bool disposing) { } } else { + // The items are taken now: DisposableModelBase clears the collection as soon as this returns, + // before the dispatcher runs the disposal, and a clear while it runs would break the enumeration. + var disposables = this.ToArray(); Application.Current.Dispatcher.InvokeAsync(() => { - foreach (var disposable in this) { + foreach (var disposable in disposables) { disposable?.Dispose(); } }, System.Windows.Threading.DispatcherPriority.Background); diff --git a/tests/MSDIAL5/MsdialGuiAppTests/Model/DataObj/AnalysisFileBeanModelTests.cs b/tests/MSDIAL5/MsdialGuiAppTests/Model/DataObj/AnalysisFileBeanModelTests.cs index 371251dac..c1b670cb2 100644 --- a/tests/MSDIAL5/MsdialGuiAppTests/Model/DataObj/AnalysisFileBeanModelTests.cs +++ b/tests/MSDIAL5/MsdialGuiAppTests/Model/DataObj/AnalysisFileBeanModelTests.cs @@ -1,4 +1,7 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; +using CompMs.App.Msdial.Utility.Tests; +using CompMs.MsdialCore.MSDec; +using CompMs.MsdialCore.Parser; +using Microsoft.VisualStudio.TestTools.UnitTesting; using System; using System.Collections.Generic; using System.IO; @@ -93,6 +96,28 @@ public void PeakListFileExistsFollowsTheFile() { Assert.IsFalse(model.PeakListFileExists()); } + [TestMethod()] + public void ReleasingTheLoaderOrDisposingTheModelClosesTheDeconvolutionFile() { + // With an application running, as in MS-DIAL, the model's disposables are disposed on its dispatcher. + TestApplication.Ensure(); + var dcl = Path.Combine(_folder, "sample.dcl"); + MsdecResultsWriter.Write(dcl, new List { new MSDecResult { ScanID = 0, PrecursorMz = 300.1, }, }); + var model = AnalysisFileBeanModelTestHelper.Create(); + model.File.DeconvolutionFilePath = dcl; + + // Reprocessing rewrites the .dcl after DatasetModel releases the loaders. + Assert.IsNotNull(model.MSDecLoader.LoadMSDecResult(0)); + Assert.IsTrue(TestFiles.IsOpen(dcl)); + model.ReleaseMSDecLoader(); + Assert.IsFalse(TestFiles.IsOpen(dcl)); + + // Closing the dataset disposes its file models, and the loader opened again goes with them. + Assert.IsNotNull(model.MSDecLoader.LoadMSDecResult(0)); + model.Dispose(); + TestApplication.WaitForBackgroundWork(); + Assert.IsFalse(TestFiles.IsOpen(dcl)); + } + private static AnalysisFileBeanModel Create(string analysisFilePath) { var model = AnalysisFileBeanModelTestHelper.Create(); model.AnalysisFilePath = analysisFilePath; diff --git a/tests/MSDIAL5/MsdialGuiAppTests/Model/Lcms/LcmsAnalysisModelWithoutRawTests.cs b/tests/MSDIAL5/MsdialGuiAppTests/Model/Lcms/LcmsAnalysisModelWithoutRawTests.cs index d490285fd..9a988b580 100644 --- a/tests/MSDIAL5/MsdialGuiAppTests/Model/Lcms/LcmsAnalysisModelWithoutRawTests.cs +++ b/tests/MSDIAL5/MsdialGuiAppTests/Model/Lcms/LcmsAnalysisModelWithoutRawTests.cs @@ -2,6 +2,7 @@ using CompMs.App.Msdial.Model.DataObj.Tests; using CompMs.App.Msdial.Model.Search; using CompMs.App.Msdial.Model.Service; +using CompMs.App.Msdial.Utility.Tests; using CompMs.Common.Components; using CompMs.Common.DataObj.Result; using CompMs.Common.Enum; @@ -25,42 +26,29 @@ namespace CompMs.App.Msdial.Model.Lcms.Tests public class LcmsAnalysisModelWithoutRawTests { private string _folder = string.Empty; + private AnalysisFileBeanModel? _file; [TestInitialize()] public void Setup() { _folder = Path.Combine(Path.GetTempPath(), $"msdial-lcms-without-raw-{Guid.NewGuid():N}"); Directory.CreateDirectory(_folder); - EnsureApplication(); - } - - // The peak spot filters are attached through Application.Current.Dispatcher, so the model needs an application - // whose dispatcher runs; it gets one on its own STA thread, as the application's UI thread would be. - private static void EnsureApplication() { - if (System.Windows.Application.Current is not null) { - return; - } - using var ready = new System.Threading.ManualResetEventSlim(); - var thread = new System.Threading.Thread(() => { - _ = new System.Windows.Application { ShutdownMode = System.Windows.ShutdownMode.OnExplicitShutdown, }; - ready.Set(); - System.Windows.Threading.Dispatcher.Run(); - }) - { - IsBackground = true, - }; - thread.SetApartmentState(System.Threading.ApartmentState.STA); - thread.Start(); - ready.Wait(); + // The peak spot filters are attached through Application.Current.Dispatcher, so the model needs an application + // whose dispatcher runs. + TestApplication.Ensure(); } [TestCleanup()] public void Cleanup() { + // The file model owns the .dcl reader that opening the sample creates, as the dataset's file collection + // does in the application, so the folder can be deleted only after it is disposed. + _file?.Dispose(); + TestApplication.WaitForBackgroundWork(); Directory.Delete(_folder, recursive: true); } [TestMethod()] public void ASampleWithoutRawDataOpensFromWhatTheProjectStores() { - var file = AnalysisFileBeanModelTestHelper.Create(); + var file = _file = AnalysisFileBeanModelTestHelper.Create(); file.AnalysisFileName = "sample"; file.AnalysisFilePath = Path.Combine(_folder, "sample.mzML"); // not written: the raw data are missing file.AcquisitionType = AcquisitionType.DDA; @@ -107,7 +95,9 @@ public void ASampleWithoutRawDataOpensFromWhatTheProjectStores() { new MsfinderSearcherFactory(dataBases, mapper, parameter, "MS-FINDER"), new MessageBroker())) { stopwatch.Stop(); - Assert.IsTrue(stopwatch.Elapsed < TimeSpan.FromSeconds(2), $"Opening took {stopwatch.Elapsed}."); + // Reading the missing raw data would wait 5 s before its first retry (BaseDataProvider), so any retry + // fails this; a cold CI runner has taken 2.8 s to open the sample without one. + Assert.IsTrue(stopwatch.Elapsed < TimeSpan.FromSeconds(5), $"Opening took {stopwatch.Elapsed}."); Assert.IsFalse(model.IsRawDataAvailable); Assert.IsNull(model.EicLoader); @@ -123,6 +113,14 @@ public void ASampleWithoutRawDataOpensFromWhatTheProjectStores() { Assert.AreEqual(RawDataGuard.EicTitle, model.EicModel.GraphTitle); Assert.AreEqual(RawDataGuard.Ms1SpectrumTitle, model.SurveyScanModel.Elements.GraphTitle); } + // The model borrows the .dcl reader from the file model, which keeps it for the next time the sample is + // opened. Once the file model is disposed too, as closing the dataset does, nothing the project stores + // may stay open. This is checked before any collection, so no finalizer closes a leaked stream first. + file.Dispose(); + TestApplication.WaitForBackgroundWork(); + Assert.IsFalse(TestFiles.IsOpen(file.File.DeconvolutionFilePath), "The .dcl is still open."); + Assert.IsFalse(TestFiles.IsOpen(file.PeakListFilePath), "The .pai2 is still open."); + GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect(); diff --git a/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestApplication.cs b/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestApplication.cs new file mode 100644 index 000000000..56c572c65 --- /dev/null +++ b/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestApplication.cs @@ -0,0 +1,38 @@ +using System.Threading; +using System.Windows; +using System.Windows.Threading; + +namespace CompMs.App.Msdial.Utility.Tests; + +/// +/// A WPF application whose dispatcher runs on its own STA thread, as the application's UI thread would be. +/// An AppDomain holds one application at most, so once a test makes it, every later test in the run has it too. +/// +internal static class TestApplication +{ + public static void Ensure() { + if (Application.Current is not null) { + return; + } + using var ready = new ManualResetEventSlim(); + var thread = new Thread(() => { + _ = new Application { ShutdownMode = ShutdownMode.OnExplicitShutdown, }; + ready.Set(); + Dispatcher.Run(); + }) + { + IsBackground = true, + }; + thread.SetApartmentState(ApartmentState.STA); + thread.Start(); + ready.Wait(); + } + + /// + /// Returns once the dispatcher has run everything queued at Background priority or above, which is where + /// DisposableCollection disposes its items while an application runs. Call it from a thread other than the dispatcher's. + /// + public static void WaitForBackgroundWork() { + Application.Current?.Dispatcher.Invoke(() => { }, DispatcherPriority.ContextIdle); + } +} diff --git a/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestFiles.cs b/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestFiles.cs new file mode 100644 index 000000000..ee564fac2 --- /dev/null +++ b/tests/MSDIAL5/MsdialGuiAppTests/Utility/TestFiles.cs @@ -0,0 +1,20 @@ +using System.IO; + +namespace CompMs.App.Msdial.Utility.Tests; + +internal static class TestFiles +{ + /// + /// Whether any handle on the file is open, in this process or another: exclusive access fails while one is. + /// A missing file still throws. + /// + public static bool IsOpen(string path) { + try { + using (File.Open(path, FileMode.Open, FileAccess.ReadWrite, FileShare.None)) { } + return false; + } + catch (IOException) when (File.Exists(path)) { + return true; + } + } +}