From e69f1847b89a2d2bfe761c6f153846876aa0acdc Mon Sep 17 00:00:00 2001 From: jonnew Date: Wed, 4 Sep 2024 16:09:54 -0400 Subject: [PATCH] Improve the shutdown behavior - This commit addresses two issues - The first is that if there was an exception setting block read or write sizes, contextConfiguration within ContextTask was not disposed. This lead to a deadlock that required the process to be restarted even if the offending parameter was changed. - When addressing this, I noticed that there may be a more general issue that is documented on line 314 of ContextTask. The general issue is that contextConfiguration must always be disposed, and there are (unlikely) ways that it might not be. - Additionally, I improved the error messages presented when a ReadSize or WriteSize exception occurred indicating what value the user needs to use. - The second issue was that when a hub was configured as a passthrough device, it was not reset to a stanard hub when acqusition was stopped. This could cause issues because the raw device used by passthroughs is a member of hub zero and could lead to huge required block read sizes even a headstage was no longer present. To address this, I simply returned an active disposable whose action is to reset the port to standard mode fro source.CoonfigureHost in ConfigurePortController, rather than Disposable.Empty, which did nothing. --- OpenEphys.Onix1/ConfigurePortController.cs | 4 +- OpenEphys.Onix1/ContextTask.cs | 65 +++++++++++++++++----- 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/OpenEphys.Onix1/ConfigurePortController.cs b/OpenEphys.Onix1/ConfigurePortController.cs index 5e5bb630..f9587c56 100644 --- a/OpenEphys.Onix1/ConfigurePortController.cs +++ b/OpenEphys.Onix1/ConfigurePortController.cs @@ -51,7 +51,9 @@ public override IObservable Process(IObservable source var portShift = ((int)deviceAddress - 1) * 2; var passthroughState = (hubConfiguration == HubConfiguration.Passthrough ? 1 : 0) << portShift; context.HubState = (PassthroughState)(((int)context.HubState & ~(1 << portShift)) | passthroughState); - return Disposable.Empty; + + // leave in standard mode when finished + return Disposable.Create(() => context.HubState = (PassthroughState)((int)context.HubState & ~(1 << portShift))); }) .ConfigureLink(context => { diff --git a/OpenEphys.Onix1/ContextTask.cs b/OpenEphys.Onix1/ContextTask.cs index 1cdc27c6..cefffa2b 100644 --- a/OpenEphys.Onix1/ContextTask.cs +++ b/OpenEphys.Onix1/ContextTask.cs @@ -255,25 +255,64 @@ internal Task StartAsync(int blockReadSize, int blockWriteSize, CancellationToke if (!acquisition.IsCompleted) throw new InvalidOperationException("Acquisition already running in the current context."); - // NB: Configure context before starting acquisition + + // NB: Configure context before starting acquisition or the the settings (e.g. Block read + // and write sizes) will not be respected var contextConfiguration = ConfigureContext(); - ctx.BlockReadSize = blockReadSize; - ctx.BlockWriteSize = blockWriteSize; - - // TODO: Stuff related to sync mode is 100% ONIX, not ONI. Therefore, in the long term, - // another place to do this separation might be needed - int address = ctx.HardwareAddress; - int mode = (address & 0x00FF0000) >> 16; - if (mode == 0) // Standalone mode + + try + { + // set block read and write size + ctx.BlockReadSize = blockReadSize; + ctx.BlockWriteSize = blockWriteSize; + + // TODO: Stuff related to sync mode is 100% ONIX, not ONI. Therefore, in the long term, + // another place to do this separation might be needed + int address = ctx.HardwareAddress; + int mode = (address & 0x00FF0000) >> 16; + if (mode == 0) // Standalone mode + { + ctx.Start(true); + } + else // If synchronized mode, reset counter independently + { + ctx.ResetFrameClock(); + ctx.Start(false); + } + + } + catch (oni.ONIException ex) when (ex.Number == -20) + { + lock (regLock) + { + ctx.Stop(); + contextConfiguration.Dispose(); + } + throw new InvalidOperationException($"The requested read size of {blockReadSize} bytes is too small for the current " + + $"hardware configuration, which requires at least {ctx.MaxReadFrameSize} bytes.", ex); + } + catch (oni.ONIException ex) when (ex.Number == -24) { - ctx.Start(true); + lock (regLock) + { + ctx.Stop(); + contextConfiguration.Dispose(); + } + throw new InvalidOperationException($"The requested write size of {blockWriteSize} bytes is too small for the current " + + $"hardware configuration, which requires at least {ctx.MaxWriteFrameSize} bytes.", ex); } - else // If synchronized mode, reset counter independently + catch { - ctx.ResetFrameClock(); - ctx.Start(false); + lock (regLock) + { + ctx.Stop(); + contextConfiguration.Dispose(); + } + throw; } + // TODO: If during the creation of of collectFramesCancellation, collectFramesToken, frameQueue, readFrames, or distributeFrames + // an exception is thrown, contextConfiguration will not be disposed. The process will need to be restarted to get out of deadlock collectFramesCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); var collectFramesToken = collectFramesCancellation.Token; var frameQueue = new BlockingCollection(MaxQueuedFrames);