Is this really an Illegal Cross Thread Call?

G

Guest

I have a Thread making a callback to a Form function that alters Form data.
Of course, the debugger is telling me, "Cross-thread operation not valid:
Control 'FormTest' accessed from a thread other than the thread it was
created on."

But I am locking before I touch the shared data, so do I actually need to
worry about this, or is it just the debugger being oversimplistic?

Specifically, I have:

public delegate void TestEvent(ref string Payload);

public class FormTest : System.Windows.Forms.Form
{
...
void Run
{
TestHarness test = new TestHarness();
test.Callback += TestFunction; // Callback is of type
TestEvent
Thread t = new Thread(new ThreadStart(test.Go));
t.IsBackground = true;
t.Start();
}

private void TestFunction(ref string Payload)
{
lock(this) WriteOut(ref Payload); // ??? Doesn't this lock
prevent thread synchronization problems???
}

private void WriteOut(ref string Payload)
{
this.Text = Payload;
}
 
M

Mattias Sjögren

Dave,
But I am locking before I touch the shared data, so do I actually need to
worry about this, or is it just the debugger being oversimplistic?

Yes the warning is right. You should only access Winforms controls
from the thread they were created on. Lock doesn't help here.


Mattias
 
L

Linda Liu [MSFT]

Hi,
Thank you for posting.
If you use multithreading to improve the performance your Windows Forms
applications, you must be careful to make calls to your controls in a
thread-safe way.

Access to Windows Forms controls is not inherently thread safe. If you have
two or more threads manipulating the state of a control, it is possible to
force the control into an inconsistent state. Other thread-related bugs are
possible as well, including race conditions and deadlocks. It is important
to ensure that access to your controls is done in a thread-safe way.

The .NET Framework helps you detect when you are accessing your controls in
a manner that is not thread safe. When you are running your application in
the debugger, and a thread other than the one which created a control
attempts to call that control, the debugger raises an
InvalidOperationException with the message, "Control control name accessed
from a thread other than the thread it was created on."

To make the access to your controls in a thread-safe way, you should use
the Invoke method of the control. The following is a sample.

delegate void WriteOutDelegate(ref string Payload);
void test_Callback(ref string Payload)
{
object[] args = new object[1]{Payload};
this.Invoke(new WriteOutDelegate(WriteOut),args);
}

private void WriteOut(ref string Payload)
{
this.Text = Payload;
}

Hope this will help you.
If you have any other concerns, or need anything else, please don't
hesitate to let me know.


Sincerely,
Linda Liu
Microsoft Online Community Support

====================================================
When responding to posts,please "Reply to Group" via
your newsreader so that others may learn and benefit
from your issue.
====================================================
 
J

Jon Skeet [C# MVP]

Dave said:
I have a Thread making a callback to a Form function that alters Form data.
Of course, the debugger is telling me, "Cross-thread operation not valid:
Control 'FormTest' accessed from a thread other than the thread it was
created on."

But I am locking before I touch the shared data, so do I actually need to
worry about this, or is it just the debugger being oversimplistic?

Yes, you need to worry about it. UI components have thread affinity -
they really, really shouldn't be touched except on their own thread.
(Aside from the calls to CreateGraphics, InvokeRequired, BeginInvoke
and Invoke.)

One further note - you appear to be passing string references by
reference without using the by-ref semantics. Is there a reason for
this? You may be unaware of how parameter passing works. See
http://www.pobox.com/~skeet/csharp/parameters.html

Jon
 
C

Chris Crowther

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
delegate void WriteOutDelegate(ref string Payload);
void test_Callback(ref string Payload)
{
object[] args = new object[1]{Payload};
this.Invoke(new WriteOutDelegate(WriteOut),args);
}

That also, rather usefuly, helped me resolve getting my head around
the SerialPinChangedEventHandler and using Invoke() when you need to
modify Form UI elements.

So thanks from me for coincidental assistance :)
Sincerely,
Linda Liu
Microsoft Online Community Support

- --
Chris Crowther
Developer
J&M Crowther Ltd.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (MingW32)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFESJh8z8bl0gyT/hERAswcAKCQJ53BUPhJL1GCFd5qfkFF6b9ZBgCeO3nN
mkl2xlQQsE/9WnHUsRJr+gA=
=22sA
-----END PGP SIGNATURE-----
 
G

Guest

So I obviously don't understand something important about locking. It looks
to me like the only way the other thread can touch the Form control is
through the TestFunction, and the TestFunction locks the entire object until
the Form text is changed. How could this possibly go wrong?

And would your answer be different if I make either TestFunction() or
WriteOut() static?
 
G

Guest

Thank you for this response. But I am obviously failing to understand what's
going on behind the scenes:

Linda Liu said:
Access to Windows Forms controls is not inherently thread safe. If you have
two or more threads manipulating the state of a control, it is possible to
force the control into an inconsistent state. Other thread-related bugs are
possible as well, including race conditions and deadlocks. It is important
to ensure that access to your controls is done in a thread-safe way.

Given that my private WriteOut() method is only called in a locked
condition, and (let us assume) is guaranteed to return, could you give an
example of any unsafe multi-threading effects that could be achieved on this
class?

To make the access to your controls in a thread-safe way, you should use
the Invoke method of the control. The following is a sample.

delegate void WriteOutDelegate(ref string Payload);
void test_Callback(ref string Payload)
{
object[] args = new object[1]{Payload};
this.Invoke(new WriteOutDelegate(WriteOut),args);
}

private void WriteOut(ref string Payload)
{
this.Text = Payload;
}

I thought Invoke is just a means of reflection. What is it about this
construct that ensures thread safety?

To me it just looks like a hideous kludge ... and I can't even understand
how it could help.
 
G

Guest

Uh oh ... it must be worse than I thought. I migrated from C++ to C# over a
year ago, and I thought I had this straight, but I don't understand what
you're saying even afer rereading your parameters.html page.

Just to be clear, in my example, every function call in the callback loop
uses "ref string" arguments. In this case I am not modifying the string
values and so the only reason to do this, I thought, was performance -- to
avoid making copies of very large strings. Thinking about it further I
suppose that since strings are immutable reference types, then they are
passed by reference anyway and I wouldn't incur any performance penalty
unless I tried to modify them, in which case a new string would be created
and I would just be causing the original string to be replaced by it.

Is this what you were referring to when you say I am "passing string
references by
reference without using the by-ref semantics," or am I committing an actual
mistake of some sort in this example?
 
M

Marc Gravell

Taking out a lock on the form is completely irrelevant to thread-affinity.
It isn't the debugger being over-simplistic: rather, the runtime (correctly)
catching your incorrect usage. What you are trying to do will not work, so
stop trying to do it. You *need* to .Invoke() or .BeginInvoke() back to the
owning thread before you update the UI. Linda's response seems pretty
comprehensive, else read MSDN / google.

And for reference, lock(this) is often a bad idea, as it allows objects code
of your scope to *without realising it* share a lock with you; a better
approach is to lock on a dedicated handy object, which you may (or may not)
choose to publicly expose for other callers to sync against; at least if
they lock(something.SyncLock) then they expect you to be using that lock
too.

Marc
 
M

Marc Gravell

Typo:
as it allows objects code of your scope should be
as it allows objects out of you scope

i.e. external code may want to sync itself using your object as the lock (no
technical reason why it shouldn't)

Marc
 
J

Jon Skeet [C# MVP]

Dave said:
Uh oh ... it must be worse than I thought. I migrated from C++ to C# over a
year ago, and I thought I had this straight, but I don't understand what
you're saying even afer rereading your parameters.html page.

Just to be clear, in my example, every function call in the callback loop
uses "ref string" arguments. In this case I am not modifying the string
values and so the only reason to do this, I thought, was performance -- to
avoid making copies of very large strings. Thinking about it further I
suppose that since strings are immutable reference types, then they are
passed by reference anyway and I wouldn't incur any performance penalty
unless I tried to modify them, in which case a new string would be created
and I would just be causing the original string to be replaced by it.

They are not passed by reference. The reference is passed by value.
It's important to understand the difference. In this case, the values
of both the argument and the parameter are references. The parameter
gets a *copy* of the value of the argument. Those are "pass by value"
semantics, even though the values involved are references.

The fact that they're immutable is irrelevant to all of the above. The
only difference that mutability makes is to whether a method can change
the contents of the object a parameter refers to, not whether changing
the value of the parameter itself (i.e. the reference) to refer to a
different object makes the caller see a different value too.
Is this what you were referring to when you say I am "passing string
references by reference without using the by-ref semantics," or am I
committing an actual mistake of some sort in this example?

You're not making use of the by-ref semantics because you're never
changing the value of the parameter. If you changed it to refer to a
different string, that change would be visible by the caller, which
would presumably be intentional - that's what I mean by "using the
by-ref semantics".

To cut a long story short (too late?) you should get rid of the "ref"
modifier - and read the page again :)

Jon
 
N

Nick Hounsome

Dave Booker said:
So I obviously don't understand something important about locking. It
looks
to me like the only way the other thread can touch the Form control is
through the TestFunction, and the TestFunction locks the entire object
until
the Form text is changed. How could this possibly go wrong?

And would your answer be different if I make either TestFunction() or
WriteOut() static?

"The other thread" is the thread that processes windows events.
It handles those events by calling OnXXXX (which might be overridden to do
anything to the form) which calls event handling delegates which MIGHT
change anything including the Text property that you are trying to access
from another thread.

locks are irrelevant because the framework doesn't lock the form when
processing events and every one has to play for locking to work.

Does anyone know exactly where and when it detects the cross thread call?
 
L

Linda Liu [MSFT]

Hi ,
Thank you for reply.
In the following sentences:
Thread t = new Thread(new ThreadStart(test.Go));
t.IsBackground = true;
t.Start();
you try to trigger the Callback event in another thread t(other than the
program main thread). Since you have subcribed the Callback event and
defined the corresponding handling method TestFunction, TestFunction will
be called. TestFunction will access FormTest by the sentence "this.Text =
Payload;" which was created by the program main thread. Thus a cross-thread
operation happens. Windows Forms controls are a little different from
general resources. They can only be accessed in the threads which have
created them.
If you replace the sentence "this.Text = Payload" with
"MessageBox.Show("hello world")" in WriteOut method, the program will run
properly.

Hope this is helpful to you.
If you have any other concerns or need anything else, please don't hesitate
to tell me.




Sincerely,
Linda Liu
Microsoft Online Community Support

====================================================
When responding to posts,please "Reply to Group" via
your newsreader so that others may learn and benefit
from your issue.
====================================================
 
L

Lucian Wischik

Dave Booker said:
Given that my private WriteOut() method is only called in a locked
condition, and (let us assume) is guaranteed to return, could you give an
example of any unsafe multi-threading effects that could be achieved on this
class?

(1) Your thread does lock()

(2) The main UI thread queries the form's text to find out how wide it
is, so it knows how to draw it. Note that the string this.Text is not
protected by a lock, so it proceeds. It calculates a width of 5.

(3) Your thread does this.Text = payload, then releases the lock.

(4) The main thread draws itself with a width of 5, even though the
text really takes up much more space.


Here's another possibility. Bear in mind that we don't really know how
the internals of form title bars are implemented. So we have to assume
the worst.

(1) Your thread does lock()

(2) Your thread wishes to set the form's text property. The way it
implements this is first by getting the address in memory where the
text is stored (in an unsafe way), then overwriting this address. It
gets as far as reading the address.

(3) Some other event in the UI thread cuases the form to have a
different address in memory for its text.

(4) Your thread now ovewrwrites an invalid address, and unlocks.
 
G

Guest

OK, so I guess the answer is:

A. Windows forms are special, in that Form controls are ultimately handled
in a thread to which I don't have ready access.

B. I shouldn't make any assumptions about how their methods or properties
are handled.

Therefore: The only multi-thread safe way to access Windows Form elements is
through a kludgy Invoke construct.

If this is correct, what would make me really happy at this point is an
explanation of:

1. How the Invoke construct outlined by Linda Liu prevents synch problems.

2. Why it isn't kludgy.

In any case, thank you all for your illuminating responses!
 
L

Lucian Wischik

(1) Your thread does (lock), then calls the form text's SETTER method.

(2) The setter method updates the text and then repaints the window.

(3) Meanwhile a "paint" event comes into the main UI thread which
starts repainting the window.


This means that there will be two concurrent threads both doing a
repaint on the form at the same time! And I'm sure they didn't write
their repainting code to allow for this.
 
R

RichS

As a useful tip, I created a "ControlHelper.cs" class that contains a
number of static functions to perform all of my UI
cross-thread/same-thread updates. It might look a bit clunky to start
with, but it saves vast amounts of time and problems when I am coding.

My file basically looks like this ( with more in it! ), when I want to
update any controls text, I can just call
"ControlHelper.UpdateControlText( control, "TEST" )":

public class ControlHelper
{
private delegate void DelegateUpdateControlText( Control
_control, string _strText );

public static void UpdateControlText( Control _control, string
_strText )
{
if ( true != _control.InvokeRequired )
{
PerformUpdateControlText( _control, _strText );
}
else
{
_control.Invoke( new DelegateUpdateControlText(
ControlHelper.PerformUpdateControlText ), new object[] { _control,
_strText } );
}
}

private static void PerformUpdateControlText( Control _control,
string _strText )
{
_control.Text = _strText;
}
}

RichS
 
L

Lucian Wischik

Dave Booker said:
A. Windows forms are special, in that Form controls are ultimately handled
in a thread to which I don't have ready access.

I don't know what "ready access to a thread" would mean, but the UI
thread is just the main thread of the application. Your Program.cs
will invoke, in the context of this main thread, the routine
"Application.Run". Its job is to take in windows messages and dispatch
them. You do have "ready access" in the sense that each of your
event-handlers will execute in the context of this thread. You could
even rip out "Application.Run" and do something else.

Therefore: The only multi-thread safe way to access Windows Form elements is
through a kludgy Invoke construct.
2. Why it isn't kludgy.

I had some experience with the "Trestle" windowing framework (i.e. an
equivalent to .net Forms, but written in the more beautiful Modula-3
language rather than C#). It tried hard to be fully threadable,
invokable from any threads. It is immensely hard to write a correct
multithreaded windowing framework!!! The framework has to deal with
bottom-up events (mouse, keyboard, network) and top-down events
(repainting, changing controls) without deadlocking. One possibility
is to make the framework completely asynchronous and have every part
of it communicate just by posting messages. This is a very ugly style
to write in, though, because you lose track of the causality in your
code, and you can't use exceptions effectively.

What they did for Trestle was to come up with locking invariants, and
write these locking invariants as mathematical formulas in the
comments above each procedure, and then they used an automated
model-checker + theorem-prover to prove that each invariant was
satisfied and therefore there were no deadlocks. It did work, but
Trestle was a very tiny windowing system with only a fraction of the
features.

I think that the complexity of multi-threaded interaction in a
windowing system really is beyond the ability of human brains to know,
figure out, understand. (For instance, you didn't
know/figureout/understand the threading implications of what you were
doing!) Therefore we REQUIRE the support of automated model-checkers
and theorem-provers before we can write such things.

The state of the art in automated model-checkers and theorem-provers
isn't yet ready for what we need. It will require advances in
model-checkers (to deal with heaps and to reduce the exponential
state-spaces that arise from concurrency). And it will require
advances in theory (to come up with compositional theories, so the
model-checking can be done per module). Like at this conference:
http://www.comp.nus.edu.sg/~abhik/svv06/

Until that time, designers are correct (not kludgy!) to come up with
simple clear principals -- like the one you ran up against -- which
guarantee correctness and which are easy to understand, even at the
expense of being inflexible.
 
A

Alvin Bruney

this is very good stuff Lucian

--

________________________
Warm regards,
Alvin Bruney [MVP ASP.NET]

[Shameless Author plug]
The O.W.C. Black Book with .NET
www.lulu.com/owc, Amazon
Professional VSTO.NET - Wrox/Wiley 2006
-------------------------------------------------------
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top