Threads and TextBox
-
Hi, still getting to know C# and recently stumbled upon a problem I'm not sure about. I have two threads, each of them updating a WinForm's TextBox. I'm using the usual InvokeRequired:
if (this.OutputConsole.InvokeRequired)
{
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text
);
}
else
{
this.OutputConsole.AppendText(text + "\r\n");
}But bad things happened when I tried to add some sort of locking to that :~ o. I just added a lock around the above mentioned code
lock(ConsoleLock)
{
// the snippet from above
}A deadlock occured, the thread that actually needs the Invoke gets the lock ("user thread"), and tries to call Invoke, "delegating" the code to the thread that created the control ("original thread"). The original thread, though, wants to write something to the TextBox too, and is stuck waiting for the lock, currently held by the user thread, which in turn keeps waiting for the original thread to process the delegated call. So far, so good. The question is: if I remove the lock, why does it still work okay... is the TextBox, in some way, thread-safe? Is it due to the way the Invoke call works (and if so, how does it work in the first place)? I don't really see how I could make it work using locks anyway (as long as I have to use the above mentioned paradigm), do you see how it could be done? thanks for your time :)!
-
Hi, still getting to know C# and recently stumbled upon a problem I'm not sure about. I have two threads, each of them updating a WinForm's TextBox. I'm using the usual InvokeRequired:
if (this.OutputConsole.InvokeRequired)
{
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text
);
}
else
{
this.OutputConsole.AppendText(text + "\r\n");
}But bad things happened when I tried to add some sort of locking to that :~ o. I just added a lock around the above mentioned code
lock(ConsoleLock)
{
// the snippet from above
}A deadlock occured, the thread that actually needs the Invoke gets the lock ("user thread"), and tries to call Invoke, "delegating" the code to the thread that created the control ("original thread"). The original thread, though, wants to write something to the TextBox too, and is stuck waiting for the lock, currently held by the user thread, which in turn keeps waiting for the original thread to process the delegated call. So far, so good. The question is: if I remove the lock, why does it still work okay... is the TextBox, in some way, thread-safe? Is it due to the way the Invoke call works (and if so, how does it work in the first place)? I don't really see how I could make it work using locks anyway (as long as I have to use the above mentioned paradigm), do you see how it could be done? thanks for your time :)!
Why do you think a lock is necessary here? Lock is necessary when accessing finite resources such as a file where you need to ensure only one thread updates it a time. With the textbox you only need to use the Invoke method it you are trying to update it from a thread other than the UI thread.
I know the language. I've read a book. - _Madmatt
-
Hi, still getting to know C# and recently stumbled upon a problem I'm not sure about. I have two threads, each of them updating a WinForm's TextBox. I'm using the usual InvokeRequired:
if (this.OutputConsole.InvokeRequired)
{
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text
);
}
else
{
this.OutputConsole.AppendText(text + "\r\n");
}But bad things happened when I tried to add some sort of locking to that :~ o. I just added a lock around the above mentioned code
lock(ConsoleLock)
{
// the snippet from above
}A deadlock occured, the thread that actually needs the Invoke gets the lock ("user thread"), and tries to call Invoke, "delegating" the code to the thread that created the control ("original thread"). The original thread, though, wants to write something to the TextBox too, and is stuck waiting for the lock, currently held by the user thread, which in turn keeps waiting for the original thread to process the delegated call. So far, so good. The question is: if I remove the lock, why does it still work okay... is the TextBox, in some way, thread-safe? Is it due to the way the Invoke call works (and if so, how does it work in the first place)? I don't really see how I could make it work using locks anyway (as long as I have to use the above mentioned paradigm), do you see how it could be done? thanks for your time :)!
Yes, the original code is thread-safe. This is how it works: - thread 1 calls the method containing your snippet, I'll call it SetText(); - it detects invoke is required, launches a windows message to the GUI thread, and waits for it to return; - the GUI thread now executes the same method SetText(), sees no invoke is required, sets the TextBox property, and returns; - this causes thread 1, still waiting inside its SetText(), to continue. Now everything the GUI thread does gets sequenced (well, not absolutely aacurate, but good enough here) through its event queue, so even if another thread (thread 2) were to call SetText, it would basically execute the same steps as listed above, and the common part, executed on the GUI thread, would be sequenced by the event queue. So any number of SetText() calls will execute in the order they get called, without them interfering amongst themselves. Adding a lock does not make it any safer; and as you discovered, if it covers the entire content of SetText(), it brings everything to a halt, as the basic concept of the construct is that SetText, when called on a thread other than the GUI thread, causes itself to be executed once more, on the GUI thread, which would be prevented by the very lock you added. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
-
Yes, the original code is thread-safe. This is how it works: - thread 1 calls the method containing your snippet, I'll call it SetText(); - it detects invoke is required, launches a windows message to the GUI thread, and waits for it to return; - the GUI thread now executes the same method SetText(), sees no invoke is required, sets the TextBox property, and returns; - this causes thread 1, still waiting inside its SetText(), to continue. Now everything the GUI thread does gets sequenced (well, not absolutely aacurate, but good enough here) through its event queue, so even if another thread (thread 2) were to call SetText, it would basically execute the same steps as listed above, and the common part, executed on the GUI thread, would be sequenced by the event queue. So any number of SetText() calls will execute in the order they get called, without them interfering amongst themselves. Adding a lock does not make it any safer; and as you discovered, if it covers the entire content of SetText(), it brings everything to a halt, as the basic concept of the construct is that SetText, when called on a thread other than the GUI thread, causes itself to be executed once more, on the GUI thread, which would be prevented by the very lock you added. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
Hi, thanks for the answer... good to know about the queue :) I think you confused the SetText (aka the snippet) thing though. Unless C# does something behind my back (not that I'd be surprised :rolleyes: but I guess it would require the CLR to detect the caller of the Invoke method and recall it, and on top of that, it would render the delegate parameter useless), SetText doesn't get called again, so the deadlock is not due to the SetText method recursively calling itself in another thread (in fact, the deadlock only happened in certain cases and required a lot of tries to replicate in each of those cases). In other words, "as the basic concept of the construct is that SetText, when called on a thread other than the GUI thread, causes itself to be executed once more, on the GUI thread" seems wrong to me :) Just going over it here so that I know I'm not mistaken...
-
Why do you think a lock is necessary here? Lock is necessary when accessing finite resources such as a file where you need to ensure only one thread updates it a time. With the textbox you only need to use the Invoke method it you are trying to update it from a thread other than the UI thread.
I know the language. I've read a book. - _Madmatt
Um, coming from C++, I would have thought calling a function from two different threads might cause some re-entrancy issues when working on a common resource (the TextBox's content they both try to update). I would have expected the output to be either interleaved ("AAAA" and "BBBB" resulting in "AABABBBA" for example) in the better case, or some variable corruption in the worse case.
-
Hi, thanks for the answer... good to know about the queue :) I think you confused the SetText (aka the snippet) thing though. Unless C# does something behind my back (not that I'd be surprised :rolleyes: but I guess it would require the CLR to detect the caller of the Invoke method and recall it, and on top of that, it would render the delegate parameter useless), SetText doesn't get called again, so the deadlock is not due to the SetText method recursively calling itself in another thread (in fact, the deadlock only happened in certain cases and required a lot of tries to replicate in each of those cases). In other words, "as the basic concept of the construct is that SetText, when called on a thread other than the GUI thread, causes itself to be executed once more, on the GUI thread" seems wrong to me :) Just going over it here so that I know I'm not mistaken...
Sorry, my reply wasn't very accurate; I should have payed more attention to the details of your code. The way I do such things typically is like so:
public void SetText(Control control, string text) {
if (control.InvokeRequired) {
control.Invoke(new ControlStringConsumer(SetText), new object[]{control, text}); // invoking itself
} else {
control.Text=text; // the "functional part", executing only on the main thread
}
}and then my earlier reply would apply. My "canonical form" as I call it here[^], has the advantage of containing the actual operation code only once. Your code, which was
if (this.OutputConsole.InvokeRequired) {
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text);
} else {
this.OutputConsole.AppendText(text + "\r\n");
}could be simplified to
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text);assuming we are NOT on the GUI thread. Using Invoke() it also causes the delegate to be executed on the GUI thread, through Windows messages, implying an automatic sequential treatment. Now I'm not sure about the lock issue you raised; it may depend on where exactly you put it, and even more on what is inside the OutputConsoleUpdater constructor. But anyway, the conclusion will be there is no need for such a lock. FWIW: you could break the automatic sequencing by using Application.DoEvents() inside some event handler; that is exactly why doing so is a very bad idea most of the time. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
-
Sorry, my reply wasn't very accurate; I should have payed more attention to the details of your code. The way I do such things typically is like so:
public void SetText(Control control, string text) {
if (control.InvokeRequired) {
control.Invoke(new ControlStringConsumer(SetText), new object[]{control, text}); // invoking itself
} else {
control.Text=text; // the "functional part", executing only on the main thread
}
}and then my earlier reply would apply. My "canonical form" as I call it here[^], has the advantage of containing the actual operation code only once. Your code, which was
if (this.OutputConsole.InvokeRequired) {
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text);
} else {
this.OutputConsole.AppendText(text + "\r\n");
}could be simplified to
this.OutputConsole.Invoke(
new OutputConsoleUpdater(delegate(string s) { this.OutputConsole.AppendText(s + "\r\n"); }),
text);assuming we are NOT on the GUI thread. Using Invoke() it also causes the delegate to be executed on the GUI thread, through Windows messages, implying an automatic sequential treatment. Now I'm not sure about the lock issue you raised; it may depend on where exactly you put it, and even more on what is inside the OutputConsoleUpdater constructor. But anyway, the conclusion will be there is no need for such a lock. FWIW: you could break the automatic sequencing by using Application.DoEvents() inside some event handler; that is exactly why doing so is a very bad idea most of the time. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
I only read formatted code with indentation, so please use PRE tags for code snippets.
I'm not participating in frackin' Q&A, so if you want my opinion, ask away in a real forum (or on my profile page).
Perfect, the idiom you use is definitely more elegant :) glad I learned something new today. As for the deadlock, I'm quite sure why it happens (the "pump" that does the Invoke requests doesn't get called as the whole thread is stuck waiting in the first place), no problems there. thanks a lot for your time! ;)