Thread safety

[TimK]

I think that the problem is that the event code is called by many threads. I want the event code to be executed by only one thread at a time. If I had replaced the code: OnEvent(EventArgs.Empty); in the first example with the following: MyForm.Text = "A Caption"; then the rules for safe thread practices would clearly be broken.

[Meysam Mahfouzi]

Here, if you have only one CPU, it's impossible for two events be invoked simultaneously!:) It seems you have misunderstood the usage of lock statement? or am I wrong?!

---

Don't forget, that's

Persian Gulf

not Arabian gulf!

[TimK]

Well you cant write code assuming only one CPU. I have dual CPUs. Perhaps I have not put my question clearly enougth. I have a method that is called from many threads. I want to ensure that only one thread has access to the method at any one time. Heres another example:

public class ThreadTest1
{
public void Start()
{
for (int i = 0; i < 100; i++)
{
Thread thread = new Thread( new ThreadStart(ThreadMethod));
thread.Start();
}
}

private void ThreadMethod()
{
for (int i = 0; i < 100; i++)
{
Testing();
}
}

private void Testing()
{
Console.WriteLine("Enter");
Thread.Sleep(10);
Console.WriteLine("Exit");
}
}

The console output ... or part of it at least, looks like this: Enter Exit Enter Enter Exit Exit The output I want is this: Enter Exit Enter Exit Enter Exit How would I modify the code above to do this?

[scadaguy]

Although the execution of multiple events cannot be performed simultaneously on a single CPU, they can certainly be interlaced since they are executing on separate threads. The usage of synchronization techniques still applies. Furthermore, making assumptions about the hardware and how it relates to thread synchronization is a dangerous road to go down. Brian

[scadaguy]

I would intentionally leave the ThreadTest class unsafe for multithreading. Inform the subscribers of this class' event that it is executed on another thread. Let the subscribers handle their own synchronization. Remember, the ThreadTest class is not safe for multithreading so that means subscribing and unsubscribing to the event is also not safe multithreading. Here's how the code might look...

public class ThreadTest
{
public event EventHandler MyEvent;

public void Start()
{
Thread thread = new Thread(new ThreadStart(this.ThreadMethod));
thread.Start();
}

private void ThreadMethod()
{
while (true)
{
Thread.Sleep(1000);
if (MyEvent != null) MyEvent(this, EventArgs.Empty);
}
}
}

public class Subscriber
{
public void Run()
{
// As a subscriber to ThreadTest.MyEvent I know that whatever
// delegate I specify for the handler will be executing on an
// unknown thread. I must take care of my own synchronization.

ThreadTest test = new ThreadTest();
test.MyEvent += new EventHandler(this.OnEvent);
test.Start();
test.Start();
test.Start();

// Continue with the whatever else you need to do here...

}

public void OnEvent(object source, EventArgs e)
{
// It is NOT safe to access shared members here.

lock (this)
{
  // It is safe to access shared members here.
}

}
}

[TimK]

Thanks for the suggestions Brian. This is just the sort of help I was after.

[TimK]

Thanks for these comments. From the comments made earlier I was starting to think that my understanding of the need for synchronization was completely wrong. Your comments have really cleared things up for me. :)

[Meysam Mahfouzi]

Hummmm, but because of one reason, you may need to use lock in your first example. Consider below situation: A thread(say thread_A) reaches here:

if (Event != null){
Event(this, e);
}

thread_A checks and sees that Event is not null, and goes to fire Event(this, e). But before calling it, its time slice terminates. Now suppose that in our main application, by using **-=** operator the Event gets null. Now it's thread_A's turn to continue. But an exception will be fired by calling Event(this, e). This will not happen be using below code:

lock(this){
if (Event != null){
Event(this, e);
}
}

I am wondered why below code doesn't lock the Event and the exception would occur:

lock(Event){
if (Event != null){
Event(this, e);
}
}

---

Don't forget, that's

Persian Gulf

not Arabian gulf!

[TimK]

Your example is interesting but I think that your suggested solution is flawed. Placing the lock around the event raising code e.g.

lock(this) {
if (Event != null) {
Event(this, e);
}
}

will not stop our main application from using -= operator on the Event. The lock places the event raising code in a critical section. It does not stop other threads accessing the Event property. What do you think?:confused:

[Meysam Mahfouzi]

I don't think;), I tried it and it worked seamlessly!:)

---

Don't forget, that's

Persian Gulf

not Arabian gulf!

[TimK]

OK ... How about posting your test code.

[scadaguy]

I mentioned this problem briefly in my original post. My example code assumes that all event handlers have been added prior to calling ThreadTest.Start(). It's easy is see that unsubscribing with -= will cause a lot of problems after the threads have been started, but += could be problematic as well. This problem is difficult solve correctly and that is why I suggested to intentionally leave the ThreadTest class unsafe for multithreading. Simply locking the code that raises the event is insufficient. You must lock the -= and += operators as well. This can be done with special syntax, using the add and remove keywords.

public class ThreadTest
{
private ArrayList _events = new ArrayList();

public event EventHandler MyEvent
{
add { lock (this) { _events.Add(value); } }
remove { lock (this) { _events.Remove(value); } }
}

public void Start()
{
// Same code as before
}

private void ThreadMethod()
{
while (true)
{
ArrayList clone;
lock (this)
{
clone = (ArrayList)_events.Clone();
}
foreach (EventHandler e in clone)
{
e.DynamicInvoke(...);
}
}
}
}

If you're going to do this you might as well make every method on ThreadTest thread-safe to avoid confusion and inconsistency. This solution still requires that subscribers handle their on synchronization. Brian

[scadaguy]

You were lucky :)

[scadaguy]

TimK wrote: What do you think? You are correct.

[Meysam Mahfouzi]

I just need your e-mail to send source code for you.

---

Don't forget, that's

Persian Gulf

not Arabian gulf!

[Meysam Mahfouzi]

Brian Gideon wrote: You were lucky I can post the source code for you and you will see you are more lucky than me after running it and having no exception! ;);P (need for your mail)

---

Don't forget, that's

Persian Gulf

not Arabian gulf!