Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions DeviceUpdateThread.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
// 2026/09/07 - Mike Pullen - Report device status through the shared status palette
// 2026/09/09 - Mike Pullen - Read the severity colours where they are shown rather than holding them, so a
// scheme changed while running is followed
// 2026/09/09 - Mike Pullen - Reported the search into the form only once its window exists, as InvokeRequired
// cannot say which thread owns a form that was never shown
//*********************************************************************************************************************
using System;
using System.ComponentModel;
Expand Down Expand Up @@ -144,7 +146,7 @@ private static void GetDevicePorts()
private static void UpdateDeviceList()
{
// Check if the parent has been set and not terminating early
if ((null != m_Parent) && (false == Terminating))
if (ParentIsReady)
{
// Check if invoke is required (should be)
if (m_Parent.InvokeRequired)
Expand All @@ -165,8 +167,8 @@ private static void UpdateDeviceList()
/// </summary>
private static void BackupInfoBox()
{
// Make sure the parent is valid and not terminating early
if ((null != m_Parent) && (false == Terminating))
// Make sure there is a window to report into
if (ParentIsReady)
{
// Get the current status box state through the parent's interface
m_Parent.GetStatusBoxState(out m_sStatusBoxText, out m_StatusBoxTextColor, out m_StatusBoxBackColor);
Expand All @@ -181,8 +183,8 @@ private static void BackupInfoBox()
/// <param name="backColor">IN - Background color to set for the info box</param>
private static void UpdateInfoBox(string sText, Color textColor, Color backColor)
{
// Make sure the parent is valid and not terminating early
if ((null != m_Parent) && (false == Terminating))
// Make sure there is a window to report into
if (ParentIsReady)
{
// Use parent's method which already handles invoke requirements
m_Parent.SetStatusBoxState(sText, textColor, backColor);
Expand All @@ -197,8 +199,8 @@ private static void UpdateInfoBox(string sText, Color textColor, Color backColor
/// </summary>
private static void RestoreInfoBox()
{
// Make sure the parent is valid and not terminating early
if ((null != m_Parent) && (false == Terminating))
// Make sure there is a window to report into
if (ParentIsReady)
{
// Get the message currently displayed in the info box
string sCurrentText;
Expand All @@ -225,8 +227,8 @@ private static void RestoreInfoBox()
/// <param name="deviceException">IN - The failure that stopped the search</param>
private static void ReportDeviceUpdateFailure(Exception deviceException)
{
// Make sure the parent is valid and not terminating early
if ((null != m_Parent) && (false == Terminating))
// Make sure there is a window to report into
if (ParentIsReady)
{
string sMessage = $"{m_sREADING_DEVICES_ERROR} {deviceException.Message}";
m_Parent.SetStatusBoxState(sMessage, StatusPalette.ErrorText, StatusPalette.ErrorBackground);
Expand All @@ -248,6 +250,20 @@ private static void ReportDeviceUpdateFailure(Exception deviceException)
/// </summary>
public static bool Terminating { get => (null != m_Parent) && (GeneratorForm.RngGuiStates.Terminating == m_Parent.State); }

/// <summary>
/// Whether there is a window to report the search into. A parent has to be set, it has to not be
/// closing, and its window has to exist: the search runs on a pool thread and reports by calling the
/// form, and every one of those calls decides how to marshal itself by asking InvokeRequired.
/// A form that has never been shown has no handle, and InvokeRequired on a handleless control
/// answers false, which reads as "already on the right thread" - so the search writes the controls
/// from the pool thread and races whoever else is using them. Nothing is displaying a form that has
/// no window, so there is nothing to report to and the result is dropped rather than forced in.
/// </summary>
private static bool ParentIsReady
{
get => ((null != m_Parent) && (false == Terminating) && m_Parent.IsHandleCreated);
}

// Synchronizaion objects
private static object m_Lock = new object();

Expand Down
66 changes: 50 additions & 16 deletions GeneratorForm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ public interface IGeneratorForm : ISynchronizeInvoke
{
BindingList<IRNGDevice> DeviceList { set; }
bool FileBrowseActive { get; set; }

// Whether the window exists yet. Background work has to ask before reporting into the form, because
// InvokeRequired cannot answer for a form that has never been shown: with no window handle there is
// no thread to compare against, so it says false, which reads as "already on the right thread" and
// is how a pool thread ends up writing controls directly. Form supplies this from Control.
bool IsHandleCreated { get; }

bool Running { get; }
Color StatusBoxBackColor { get; set; }
string StatusBoxText { get; set; }
Expand Down Expand Up @@ -143,22 +150,11 @@ public GeneratorForm(IRNGSessionData sessionData, IRNGDeviceTimer timer)
m_StatusLabel.BackColor = StatusPalette.NormalBackground;
m_StatusIndicator.ForeColor = StatusPalette.NormalText;

// Start the device update thread and trigger an update
DeviceUpdateThread.Parent = this;
ThreadPool.QueueUserWorkItem(state =>
{
m_DeviceUpdateComplete.Reset(); // Clear the device update complete flag
try
{
DeviceUpdateThread.ThreadProc(state);
}
finally
{
// Signal that the device update has finished however it ended, so a failure does not
// leave the close waiting for an update that will never report itself complete
m_DeviceUpdateComplete.Set();
}
});
// The device search is not started here. It reports what it finds by calling this form, and the
// form cannot be called until its window exists, so it is started from OnHandleCreated instead.
// Starting it here raced the window into being: a search that finished first had nowhere to
// report to and its results were dropped, leaving the port list empty until a device was
// plugged or unplugged.

// Create the source from the list of device ports
m_DeviceBindingSource = new BindingSource();
Expand Down Expand Up @@ -970,6 +966,30 @@ private void OnDataPointAdded(double fDataPoint, double fCurrentAverage)
}
}

/// <summary>
/// Points the device search at this form and runs one, on the thread pool so the window is not held
/// while WMI is asked what is attached. Called once the window exists, because that is what the
/// search needs in order to report back.
/// </summary>
private void StartDeviceSearch()
{
DeviceUpdateThread.Parent = this;
ThreadPool.QueueUserWorkItem(state =>
{
m_DeviceUpdateComplete.Reset(); // Clear the device update complete flag
try
{
DeviceUpdateThread.ThreadProc(state);
}
finally
{
// Signal that the device update has finished however it ended, so a failure does not
// leave the close waiting for an update that will never report itself complete
m_DeviceUpdateComplete.Set();
}
});
}

/// <summary>
/// Initializes the thread pool
/// </summary>
Expand Down Expand Up @@ -1325,6 +1345,20 @@ private void ApplyTheme()
m_ResultHistogramChart.ApplyPalette();
}

/// <summary>
/// Event handler for the window being created, which is when the device search can first be run
/// </summary>
/// <param name="e">IN - The event arguments (not used)</param>
protected override void OnHandleCreated(EventArgs e)
{
base.OnHandleCreated(e);

// The search runs on a pool thread and reports what it finds by calling this form, so it cannot
// start until there is a window for it to call. Started from the constructor it raced the window
// into being, and a search that got there first had its results dropped.
StartDeviceSearch();
}
Comment on lines +1356 to +1360

/// <summary>
/// Event handler for the system colour scheme being changed while the application is running
/// </summary>
Expand Down
40 changes: 40 additions & 0 deletions tests/RandomNumberGenerator.Test/DeviceUpdateThread.Test.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ public void Parent_SetAndGetProperty_WorksCorrectly()
// Create mock parent form
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);

Comment on lines +135 to +139
//**************************************************************//
// Act
//**************************************************************//
Expand Down Expand Up @@ -163,6 +168,11 @@ public void Terminating_ParentNotTerminating_ReturnsFalse()

// Create mock parent form with non-terminating state
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);

// Set the parent
Expand Down Expand Up @@ -196,6 +206,11 @@ public void Terminating_ParentTerminating_ReturnsTrue()

// Create mock parent form with terminating state
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Terminating);

// Set the parent
Expand Down Expand Up @@ -258,6 +273,11 @@ public void ThreadProc_ValidParent_UpdatesDeviceListAndManagesStatusBox()

// Create mock parent form
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);
mockParent.Setup(mock => mock.InvokeRequired).Returns(true);
mockParent.Setup(mock => mock.Invoke(It.IsAny<Action>())).Callback<Action>(action => action());
Expand Down Expand Up @@ -346,6 +366,11 @@ public void ThreadProc_ParentNoInvokeRequired_UpdatesDirectly()

// Create mock parent form that doesn't require invoke
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);
mockParent.Setup(mock => mock.InvokeRequired).Returns(false);

Expand Down Expand Up @@ -398,6 +423,11 @@ public void ThreadProc_ConcurrentCalls_RespectsSynchronization()

// Create mock parent form
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);
mockParent.Setup(mock => mock.InvokeRequired).Returns(false);

Expand Down Expand Up @@ -491,6 +521,11 @@ public void ThreadProc_InfoBoxUnchanged_RestoresPreviousMessage()

// Create mock parent form
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);
mockParent.Setup(mock => mock.InvokeRequired).Returns(false);

Expand Down Expand Up @@ -550,6 +585,11 @@ public void ThreadProc_InfoBoxChangedDuringUpdate_NewerMessageKept()

// Create mock parent form
var mockParent = new Mock<IGeneratorForm>();

// Stand in for a form that has been shown. The search only reports into a form whose window
// exists, because it decides how to marshal by asking the form, and a form with no window
// cannot answer that question.
mockParent.Setup(mock => mock.IsHandleCreated).Returns(true);
mockParent.Setup(mock => mock.State).Returns(GeneratorForm.RngGuiStates.Idle);
mockParent.Setup(mock => mock.InvokeRequired).Returns(false);

Expand Down