diff --git a/DeviceUpdateThread.cs b/DeviceUpdateThread.cs index e70bed4..26c6052 100644 --- a/DeviceUpdateThread.cs +++ b/DeviceUpdateThread.cs @@ -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; @@ -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) @@ -165,8 +167,8 @@ private static void UpdateDeviceList() /// 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); @@ -181,8 +183,8 @@ private static void BackupInfoBox() /// IN - Background color to set for the info box 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); @@ -197,8 +199,8 @@ private static void UpdateInfoBox(string sText, Color textColor, Color backColor /// 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; @@ -225,8 +227,8 @@ private static void RestoreInfoBox() /// IN - The failure that stopped the search 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); @@ -248,6 +250,20 @@ private static void ReportDeviceUpdateFailure(Exception deviceException) /// public static bool Terminating { get => (null != m_Parent) && (GeneratorForm.RngGuiStates.Terminating == m_Parent.State); } + /// + /// 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. + /// + private static bool ParentIsReady + { + get => ((null != m_Parent) && (false == Terminating) && m_Parent.IsHandleCreated); + } + // Synchronizaion objects private static object m_Lock = new object(); diff --git a/GeneratorForm.cs b/GeneratorForm.cs index f9fbf7b..319ec84 100644 --- a/GeneratorForm.cs +++ b/GeneratorForm.cs @@ -46,6 +46,13 @@ public interface IGeneratorForm : ISynchronizeInvoke { BindingList 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; } @@ -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(); @@ -970,6 +966,30 @@ private void OnDataPointAdded(double fDataPoint, double fCurrentAverage) } } + /// + /// 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. + /// + 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(); + } + }); + } + /// /// Initializes the thread pool /// @@ -1325,6 +1345,20 @@ private void ApplyTheme() m_ResultHistogramChart.ApplyPalette(); } + /// + /// Event handler for the window being created, which is when the device search can first be run + /// + /// IN - The event arguments (not used) + 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(); + } + /// /// Event handler for the system colour scheme being changed while the application is running /// diff --git a/tests/RandomNumberGenerator.Test/DeviceUpdateThread.Test.cs b/tests/RandomNumberGenerator.Test/DeviceUpdateThread.Test.cs index 60c16ca..7b1782a 100644 --- a/tests/RandomNumberGenerator.Test/DeviceUpdateThread.Test.cs +++ b/tests/RandomNumberGenerator.Test/DeviceUpdateThread.Test.cs @@ -132,6 +132,11 @@ public void Parent_SetAndGetProperty_WorksCorrectly() // Create mock parent form var mockParent = new Mock(); + // 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); + //**************************************************************// // Act //**************************************************************// @@ -163,6 +168,11 @@ public void Terminating_ParentNotTerminating_ReturnsFalse() // Create mock parent form with non-terminating state var mockParent = new Mock(); + + // 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 @@ -196,6 +206,11 @@ public void Terminating_ParentTerminating_ReturnsTrue() // Create mock parent form with terminating state var mockParent = new Mock(); + + // 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 @@ -258,6 +273,11 @@ public void ThreadProc_ValidParent_UpdatesDeviceListAndManagesStatusBox() // Create mock parent form var mockParent = new Mock(); + + // 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())).Callback(action => action()); @@ -346,6 +366,11 @@ public void ThreadProc_ParentNoInvokeRequired_UpdatesDirectly() // Create mock parent form that doesn't require invoke var mockParent = new Mock(); + + // 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); @@ -398,6 +423,11 @@ public void ThreadProc_ConcurrentCalls_RespectsSynchronization() // Create mock parent form var mockParent = new Mock(); + + // 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); @@ -491,6 +521,11 @@ public void ThreadProc_InfoBoxUnchanged_RestoresPreviousMessage() // Create mock parent form var mockParent = new Mock(); + + // 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); @@ -550,6 +585,11 @@ public void ThreadProc_InfoBoxChangedDuringUpdate_NewerMessageKept() // Create mock parent form var mockParent = new Mock(); + + // 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);