Skip to content

Commit e69f184

Browse files
committed
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.
1 parent 0fcdd85 commit e69f184

File tree

2 files changed

+55
-14
lines changed

2 files changed

+55
-14
lines changed

OpenEphys.Onix1/ConfigurePortController.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ public override IObservable<ContextTask> Process(IObservable<ContextTask> source
5151
var portShift = ((int)deviceAddress - 1) * 2;
5252
var passthroughState = (hubConfiguration == HubConfiguration.Passthrough ? 1 : 0) << portShift;
5353
context.HubState = (PassthroughState)(((int)context.HubState & ~(1 << portShift)) | passthroughState);
54-
return Disposable.Empty;
54+
55+
// leave in standard mode when finished
56+
return Disposable.Create(() => context.HubState = (PassthroughState)((int)context.HubState & ~(1 << portShift)));
5557
})
5658
.ConfigureLink(context =>
5759
{

OpenEphys.Onix1/ContextTask.cs

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -255,25 +255,64 @@ internal Task StartAsync(int blockReadSize, int blockWriteSize, CancellationToke
255255
if (!acquisition.IsCompleted)
256256
throw new InvalidOperationException("Acquisition already running in the current context.");
257257

258-
// NB: Configure context before starting acquisition
258+
259+
// NB: Configure context before starting acquisition or the the settings (e.g. Block read
260+
// and write sizes) will not be respected
259261
var contextConfiguration = ConfigureContext();
260-
ctx.BlockReadSize = blockReadSize;
261-
ctx.BlockWriteSize = blockWriteSize;
262-
263-
// TODO: Stuff related to sync mode is 100% ONIX, not ONI. Therefore, in the long term,
264-
// another place to do this separation might be needed
265-
int address = ctx.HardwareAddress;
266-
int mode = (address & 0x00FF0000) >> 16;
267-
if (mode == 0) // Standalone mode
262+
263+
try
264+
{
265+
// set block read and write size
266+
ctx.BlockReadSize = blockReadSize;
267+
ctx.BlockWriteSize = blockWriteSize;
268+
269+
// TODO: Stuff related to sync mode is 100% ONIX, not ONI. Therefore, in the long term,
270+
// another place to do this separation might be needed
271+
int address = ctx.HardwareAddress;
272+
int mode = (address & 0x00FF0000) >> 16;
273+
if (mode == 0) // Standalone mode
274+
{
275+
ctx.Start(true);
276+
}
277+
else // If synchronized mode, reset counter independently
278+
{
279+
ctx.ResetFrameClock();
280+
ctx.Start(false);
281+
}
282+
283+
}
284+
catch (oni.ONIException ex) when (ex.Number == -20)
285+
{
286+
lock (regLock)
287+
{
288+
ctx.Stop();
289+
contextConfiguration.Dispose();
290+
}
291+
throw new InvalidOperationException($"The requested read size of {blockReadSize} bytes is too small for the current " +
292+
$"hardware configuration, which requires at least {ctx.MaxReadFrameSize} bytes.", ex);
293+
}
294+
catch (oni.ONIException ex) when (ex.Number == -24)
268295
{
269-
ctx.Start(true);
296+
lock (regLock)
297+
{
298+
ctx.Stop();
299+
contextConfiguration.Dispose();
300+
}
301+
throw new InvalidOperationException($"The requested write size of {blockWriteSize} bytes is too small for the current " +
302+
$"hardware configuration, which requires at least {ctx.MaxWriteFrameSize} bytes.", ex);
270303
}
271-
else // If synchronized mode, reset counter independently
304+
catch
272305
{
273-
ctx.ResetFrameClock();
274-
ctx.Start(false);
306+
lock (regLock)
307+
{
308+
ctx.Stop();
309+
contextConfiguration.Dispose();
310+
}
311+
throw;
275312
}
276313

314+
// TODO: If during the creation of of collectFramesCancellation, collectFramesToken, frameQueue, readFrames, or distributeFrames
315+
// an exception is thrown, contextConfiguration will not be disposed. The process will need to be restarted to get out of deadlock
277316
collectFramesCancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
278317
var collectFramesToken = collectFramesCancellation.Token;
279318
var frameQueue = new BlockingCollection<oni.Frame>(MaxQueuedFrames);

0 commit comments

Comments
 (0)