Skip to content

Commit

Permalink
Improve the shutdown behavior
Browse files Browse the repository at this point in the history
- 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.
jonnew committed Sep 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 0fcdd85 commit e69f184
Showing 2 changed files with 55 additions and 14 deletions.
4 changes: 3 additions & 1 deletion OpenEphys.Onix1/ConfigurePortController.cs
Original file line number Diff line number Diff line change
@@ -51,7 +51,9 @@ public override IObservable<ContextTask> Process(IObservable<ContextTask> 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 =>
{
65 changes: 52 additions & 13 deletions OpenEphys.Onix1/ContextTask.cs
Original file line number Diff line number Diff line change
@@ -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<oni.Frame>(MaxQueuedFrames);

0 comments on commit e69f184

Please sign in to comment.