Skip to content

Conversation

@SteffenSH
Copy link
Contributor

Fixed System.InvalidOperationException being thrown with message: Cross-thread operation not valid: Control 'myRichTextBox' accessed from a thread other than the thread it was created on..

This would occur when a log messages was emitted from a non-UI-thread during Form construction, before the window handle was created. In these situations it is not sufficient to only check the richTextBox.InvokeRequired property as this would always return false - no matter what thread its on.

Exception type: System.InvalidOperationException
Message: "Cross-thread operation not valid: Control 'myRichTextBox' accessed from a thread other than the thread it was created on."
Source: "System.Windows.Forms"
StackTrace:

   at System.Windows.Forms.Control.get_Handle()
   at System.Windows.Forms.Control.Windows.Win32.Foundation.IHandle<Windows.Win32.Foundation.HWND>.get_Handle()
   at Windows.Win32.PInvoke.GetParent[T](T hwnd)
   at System.Windows.Forms.Control.SetParentHandle(HWND value)
   at System.Windows.Forms.Control.CreateControl(Boolean ignoreVisible)
   at System.Windows.Forms.Control.CreateControl(Boolean ignoreVisible)
   at System.Windows.Forms.Control.CreateControl()
   at System.Windows.Forms.Control.OnVisibleChanged(EventArgs e)
   at System.Windows.Forms.Control.OnVisibleChanged(EventArgs e)
   at System.Windows.Forms.Form.OnVisibleChanged(EventArgs e)
   at System.Windows.Forms.Control.SetVisibleCore(Boolean value)
   at System.Windows.Forms.Form.SetVisibleCore(Boolean value)
   at System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(msoloop reason, ApplicationContext context)
   at System.Windows.Forms.Application.ThreadContext.RunMessageLoop(msoloop reason, ApplicationContext context)
   at MyApplication.Program.Main()

@vonhoff
Copy link
Owner

vonhoff commented Jul 6, 2025

I think this approach might have some potential issues. The ManualResetEventSlim approach could potentially deadlock if called from the UI thread before the handle is created.

A simpler approach might be:

if (!richTextBox.IsHandleCreated || richTextBox.InvokeRequired)
{
    richTextBox.BeginInvoke(new Action(() => SetRtfInternal(richTextBox, rtf, autoScroll)));
    return;
}

I think this would ensure handle creation always happens on the UI thread via BeginInvoke, avoid blocking the calling thread.

@SteffenSH
Copy link
Contributor Author

At first I tried your suggestion as it sounds like a better and simple solution:

if (!richTextBox.IsHandleCreated || richTextBox.InvokeRequired)

...but it results in an exception too:

System.InvalidOperationException
  Message=Invoke or BeginInvoke cannot be called on a control until the window handle has been created.

I agree that my suggested solution in this PR must be carefully handled to avoid deadlocks if the code is evolved incorrectly. But currently I don't see any deadlocks. Because the only place making a call to SetRtf(...) is from the ProcessMessages(...) method which again runs on its own dedicated thread started like this:

_processingTask = Task.Run(() => ProcessMessages(_tokenSource.Token));

Or how would you think a deadlock could occur?
I still think my solution is the best so far.

@SteffenSH
Copy link
Contributor Author

SteffenSH commented Jul 6, 2025

Just a quick follow up on possible deadlocks... Your ConcurrentCircularBuffer is not locking anything as it just starts dropping LogEvent objects when it is full.
You code is handling disposal of the RichTextBoxSink, but it is not taking any action if the RichTextBox is disposed, e.g. the ProcessMessages() would still keep on running. I don't think its a very big deal, but there is room for improvement there (to free up the thread if the RichTextBox is disposed). That would also benefit the code suggested in this PR.
And another thing to consider would be to make the ProcessMessages() method asynch so that it don't need its own dedicated thread (I'm not sure if I would make that a high priority though to fix).

@SteffenSH
Copy link
Contributor Author

Fixed a problem where the ProcessMessages background thread could be hanging in a wait for the HandleCreated event, while at the same time the RichTextBoxSink was trying to dispose itself.

…Cross-thread operation not valid: Control 'myRichTextBox' accessed from a thread other than the thread it was created on.".

This would occur when a log messages was emitted from a non-UI-thread during Form construction, before the window handle was created.
In these situations it is not sufficient to only check the richTextBox.InvokeRequired property as this would always return false - no matter what thread its on.
@SteffenSH SteffenSH force-pushed the fix-InvalidOperationException-Cross-thread branch from 46fc0f8 to fc4f7eb Compare July 6, 2025 21:50
@vonhoff vonhoff changed the base branch from master to v3.1.2 July 7, 2025 07:04
@vonhoff vonhoff merged commit d6f5db3 into vonhoff:v3.1.2 Jul 7, 2025
1 check passed
@SteffenSH
Copy link
Contributor Author

Thank you @vonhoff for your rapid evaluation and release of this PR to NuGet. Keep up the good work! 😊👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants