c# Thread problem

[cty0000]

I have some question..
This is my first time to use thread..
Following code does not have error but two warring

The warring is

Warning 2 'System.Threading.Thread.Suspend()' is obsolete:
'Thread.Suspend has been deprecated. Please use other classes in
System.Threading, such as Monitor, Mutex, Event, and Semaphore, to
synchronize Threads or protect resources. http://go.microsoft.com/fwlink/?linkid=14202'

The warring are happened thread Resume/Suspend..
I'm not sure what is problem becuase thread programing is first time
for me...


This class to update message (GuiMsg) after received new message from
other class.
1. First application start, thread is resume by threadJob because
there are no message on QUEUE
2. If new message arrived, addNewMsg push new message to QUEUE
3. Resume thread to procss message(update)
4. Loop threadJob until QUEUE empty (during update, QUEUE can add new
msg)

===========================

Somewhere define and allowcate thread like flowing 3 line
Thread guiUpdateThread;
guiUpdateThread = new Thread(new ThreadStart(threadJob));
guiUpdateThread.Start();

public void addNewMsg(GUIMsg msg)
{
Monitor.Enter(GuiMsgQueue);
GuiMsgQueue.Enqueue(msg);
Monitor.Exit(GuiMsgQueue);

if (guiUpdateThread.ThreadState ==
ThreadState.Suspended)
guiUpdateThread.Resume();
}

private void threadJob()
{
GUIMsg gMsg;

while (true)
{
if (GuiMsgQueue.Count > 0)
{
Monitor.Enter(GuiMsgQueue);
gMsg = GuiMsgQueue.Dequeue();
Monitor.Exit(GuiMsgQueue);

update(gMsg);
}
else
{
if (guiUpdateThread.ThreadState ==
ThreadState.Running)
guiUpdateThread.Suspend();
}
}
}


Please help me..

 

[Jon Skeet [C# MVP]]

I have some question..
This is my first time to use thread..
Following code does not have error but two warring

The warring is

Warning 2 'System.Threading.Thread.Suspend()' is obsolete:
'Thread.Suspend has been deprecated. Please use other classes in
System.Threading, such as Monitor, Mutex, Event, and Semaphore, to
synchronize Threads or protect resources. http://go.microsoft.com/fwlink/?linkid=14202'

The warring are happened thread Resume/Suspend..
I'm not sure what is problem becuase thread programing is first time
for me...

As it says, you shouldn't use Suspend/Resume, basically. If you need a
producer/consumer queue (which it looks like this is) you can use Auto/
ManualResetEvent, or Monitor.Wait/Pulse/PulseAll. See the second half
of
http://pobox.com/~skeet/csharp/threads/deadlocks.shtml for an example.

You also shouldn't manually be calling Monitor.Enter/Exit - use lock
instead.

Finally, making a UI thread block at all is a bad idea - you won't be
able to do things like move the window while it's blocked. Instead,
you should either work asynchronously or have another thread waiting
for messages and calling Control.BeginInvoke or Control.Invoke to use
that message on the UI thread.

Jon

 

[cty0000]

As it says, you shouldn't use Suspend/Resume, basically. If you need a
producer/consumer queue (which it looks like this is) you can use Auto/
ManualResetEvent, or Monitor.Wait/Pulse/PulseAll. See the second half
ofhttp://pobox.com/~skeet/csharp/threads/deadlocks.shtmlfor an example.

You also shouldn't manually be calling Monitor.Enter/Exit - use lock
instead.

Finally, making a UI thread block at all is a bad idea - you won't be
able to do things like move the window while it's blocked. Instead,
you should either work asynchronously or have another thread waiting
for messages and calling Control.BeginInvoke or Control.Invoke to use
that message on the UI thread.

Jon

I changed code as your recommand...
The warring was disappear but some time it happen deadlock... -_-;;
deadlock position is lock (queueLock) at addNewMsg function...

only one difference between my code and the code which is linked at
http://pobox.com/~skeet/csharp/threads/deadlocks.shtml
is that my code need to loop forever until application finish..
So I add while (true) at threadJob... with sleep 0.1 secs in every
loop..

Could you check my code..

And can I ask one more?
My application run with one Main Form first, and button click on the
Main form then start receive form which is include thread job..

When I close Main Form, then the applicaion is not finished..
As I debug.. the Application is wait Monitor.Wait(queueLock) at
threadJob function forever
Do you know the reason...




Thread guiUpdateThread;
guiUpdateThread = new Thread(new ThreadStart(threadJob));
guiUpdateThread.Start();
readonly object queueLock = new object();

public void addNewMsg(GUIMsg msg)
{
lock (queueLock)
{
GuiMsgQueue.Enqueue(msg);
Monitor.Pulse(queueLock);
}
}

public void threadJob()
{
GUIMsg gMsg;

while (true)
{
Thread.Sleep(100);
lock (queueLock)
{
while (GuiMsgQueue.Count == 0)
Monitor.Wait(queueLock);

gMsg = GuiMsgQueue.Dequeue();
update(gMsg);
}
}
}

 

[=?ISO-8859-1?Q?G=F6ran_Andersson?=]

I changed code as your recommand...
The warring was disappear but some time it happen deadlock... -_-;;
deadlock position is lock (queueLock) at addNewMsg function...

only one difference between my code and the code which is linked at
http://pobox.com/~skeet/csharp/threads/deadlocks.shtml
is that my code need to loop forever until application finish..
So I add while (true) at threadJob... with sleep 0.1 secs in every
loop..

Could you check my code..

And can I ask one more?
My application run with one Main Form first, and button click on the
Main form then start receive form which is include thread job..

When I close Main Form, then the applicaion is not finished..
As I debug.. the Application is wait Monitor.Wait(queueLock) at
threadJob function forever
Do you know the reason...




Thread guiUpdateThread;
guiUpdateThread = new Thread(new ThreadStart(threadJob));
guiUpdateThread.Start();
readonly object queueLock = new object();

public void addNewMsg(GUIMsg msg)
{
lock (queueLock)
{
GuiMsgQueue.Enqueue(msg);
Monitor.Pulse(queueLock);
}
}

public void threadJob()
{
GUIMsg gMsg;

while (true)
{
Thread.Sleep(100);
lock (queueLock)
{
while (GuiMsgQueue.Count == 0)
Monitor.Wait(queueLock);

Oops. You have a loop inside the lock waiting for something to get into
the queue, but the only code that puts something in the queue is also in
a lock, so that will never happen. This thread will wait in this loop
forever, running at 100% CPU while doing so.

 

[Tom Spink]

I changed code as your recommand...
The warring was disappear but some time it happen deadlock... -_-;;
deadlock position is lock (queueLock) at addNewMsg function...

only one difference between my code and the code which is linked at
http://pobox.com/~skeet/csharp/threads/deadlocks.shtml
is that my code need to loop forever until application finish..
So I add while (true) at threadJob... with sleep 0.1 secs in every
loop..

Could you check my code..

And can I ask one more?
My application run with one Main Form first, and button click on the
Main form then start receive form which is include thread job..

<snippedy>

Hi,

The reason you're getting a deadlock is because you're waiting on a lock
that you already hold. It looks like you need to implement the
Producer/Consumer pattern here, as has been mentioned. Take a look here
for an example:

http://www.betasafe.com/code?view=prcon

 

[Christof Nordiek]

while (true)
{
Thread.Sleep(100);
lock (queueLock)
{
while (GuiMsgQueue.Count == 0)
Monitor.Wait(queueLock);

gMsg = GuiMsgQueue.Dequeue();

The following line should be outside of the lock block. The lock should only
spawn the deqeueing, not what is done with the deqeued object after.

update(gMsg);
}
}
}

I'm not sure, if this causes the deadlock. Since deadlocks can only occur
with atleast two lock-objects, there must something else go on. Maybe the
call on update blocks, waiting for the Mainthread, while this is trying to
achieve a lock on qeueLock. This would produce a deadlock.

Christof

 

[Jon Skeet [C# MVP]]

The reason you're getting a deadlock is because you're waiting on a lock
that you already hold.

No, that's the *only* place you can call Wait. If you call
Monitor.Wait on a lock you don't already hold, an exception will be
thrown.

It looks like you need to implement the
Producer/Consumer pattern here, as has been mentioned. Take a look here
for an example:

http://www.betasafe.com/code?view=prcon

Note that that code *also* calls Wait on a lock it already holds.

Jon

 

[Jon Skeet [C# MVP]]

On Jul 4, 11:34 am, (e-mail address removed) wrote:

I changed code as your recommand...
The warring was disappear but some time it happen deadlock... -_-;;
deadlock position is lock (queueLock) at addNewMsg function...

only one difference between my code and the code which is linked athttp://pobox.com/~skeet/csharp/threads/deadlocks.shtml
is that my code need to loop forever until application finish..
So I add while (true) at threadJob... with sleep 0.1 secs in every
loop..

Could you check my code..

The code looks mostly okay, but you haven't shown what update() does.
I don't think there's any need for you to hold the queue lock while
you do that. (I'd also put the queue into a different class to keep
the responsibilities clear.)

Jon

 

[Jon Skeet [C# MVP]]

Oops. You have a loop inside the lock waiting for something to get into
the queue, but the only code that puts something in the queue is also in
a lock, so that will never happen. This thread will wait in this loop
forever, running at 100% CPU while doing so.

No it won't - it will wait for something to pulse the monitor.
Monitor.Wait releases the lock until it's pulsed, at which point it
then waits to reacquire it before returning.

Jon

 

[Jon Skeet [C# MVP]]

I'm not sure, if this causes the deadlock. Since deadlocks can only occur
with atleast two lock-objects, there must something else go on. Maybe the
call on update blocks, waiting for the Mainthread, while this is trying to
achieve a lock on qeueLock. This would produce a deadlock.

There don't have to be two explicit locks in the code though. Suppose
the update(gMsg) code included a call to Control.Invoke, and at the
time the UI was waiting to enqueue an extra message... that would
cause a deadlock, certainly.

Jon

 

[Tom Spink]

No, that's the *only* place you can call Wait. If you call
Monitor.Wait on a lock you don't already hold, an exception will be
thrown.


Note that that code *also* calls Wait on a lock it already holds.

Jon

Whoops.

 

[Peter Duniho]

There don't have to be two explicit locks in the code though. Suppose
the update(gMsg) code included a call to Control.Invoke, and at the
time the UI was waiting to enqueue an extra message... that would
cause a deadlock, certainly.

And in fact, in my experience using Invoke() with a lock is the most
common source of deadlock in .NET code.

 

[cty0000]

And in fact, in my experience using Invoke() with a lock is the most
common source of deadlock in .NET code.

Actually I'm using invoke() to update control like textBox, Label with
lock...
According to debugger, deadlock may happen by both of invoke to
update label and lock for push a data to QUEUE
I need to update lable and also need to receive message by using
thread..

Is there any solution?

 

[Jon Skeet [C# MVP]]

Actually I'm using invoke() to update control like textBox, Label with
lock...
According to debugger, deadlock may happen by both of invoke to
update label and lock for push a data to QUEUE
I need to update lable and also need to receive message by using
thread..

Is there any solution?

Yes:

1) Pull the call to update out of the lock block. (This would also
naturally occur as part of refactoring the queue to its own task-
agnostic class.)
2) Use BeginInvoke instead of Invoke.

Jon

 

[cty0000]

Yes:

1) Pull the call to update out of the lock block. (This would also
naturally occur as part of refactoring the queue to its own task-
agnostic class.)
2) Use BeginInvoke instead of Invoke.

Jon- -

- -

I can not understand all ^^
I need to update function call during exist QUEUE data.. to do this,
updata must be lock block..
get the data until QUEUE empty and update GUI screen.
if update function pull out of lock block.. hmmm.... Actually I do not
know how to implement the code..

If I use begininvoke instead of invoke, I do not need to pull out
upate() from lock block??
I will try to find out about begininvoke..

It's Big problem for me -.-;;

 

[Jon Skeet [C# MVP]]

I can not understand all ^^
I need to update function call during exist QUEUE data.. to do this,
updata must be lock block..

No - you *fetch* the item while the queue is locked, but after you've
fetched the update you don't need to hold the lock any more. At least,
if you think you do, please explain why.

get the data until QUEUE empty and update GUI screen.
if update function pull out of lock block.. hmmm.... Actually I do not
know how to implement the code..

Just move the call to "update" to after the brace ending the lock.

If I use begininvoke instead of invoke, I do not need to pull out
upate() from lock block??
I will try to find out about begininvoke..

It's Big problem for me -.-;;

You don't *have* to pull update() out of the lock block, but you
should anyway because conceptually you don't need the lock while
you're updating.

Jon

 

[cty0000]

No - you *fetch* the item while the queue is locked, but after you've
fetched the update you don't need to hold the lock any more. At least,
if you think you do, please explain why.


Just move the call to "update" to after the brace ending the lock.



You don't *have* to pull update() out of the lock block, but you
should anyway because conceptually you don't need the lock while
you're updating.

Jon

I have changed my code like follow to test. Is this what you means?
The following code...
Launch one thread when new message arrived, then the thread aborted
after update all by using QUEUE message..
Of course more then one messages can be exist in QUEUE when threadJob
is started..
That's whay I use while statement in threadJob..

One msg in QUEUE used to update GUI screen.. (one message = one update
GUI)
So.....
I used update in while loop and lock block....


The following code also makes deadlock... at same position (invoke
control and lock in addNewMsg)
Is it possible to pull update out from lock in my case???



public void addNewMsg(GUIMsg msg)
{

lock (queueLock)
{
GuiMsgQueue.Enqueue(msg);
}

Thread guiUpdateThread = new Thread(new
ThreadStart(threadJob));
guiUpdateThread.Start();


}

private void threadJob()
{
GUIMsg gMsg;

lock (queueLock)
{
while (GuiMsgQueue.Count != 0)
{
gMsg = GuiMsgQueue.Dequeue();
update(gMsg);
}
}
}

 

[Jon Skeet [C# MVP]]

I have changed my code like follow to test. Is this what you means?

No - the call to update(gMsg) is still within the lock, and you've
effectively removed all the producer/consumer stuff. All that was
needed was a one line change, moving the call to update to one line
later.

Jon

 

[Christof Nordiek]

What he meant is, change youre original threadJob:

public void threadJob()
{
GUIMsg gMsg;

while (true)
{
Thread.Sleep(100);
lock (queueLock)
{
while (GuiMsgQueue.Count == 0)
Monitor.Wait(queueLock);

gMsg = GuiMsgQueue.Dequeue();
}
update(gMsg); //This line is moved.
}
}

After the GuiMsgQueue.Dequeue() there is no need to hold the lock.

From your code:

private void threadJob()
{
GUIMsg gMsg;

lock (queueLock)
{
while (GuiMsgQueue.Count != 0)
{
gMsg = GuiMsgQueue.Dequeue();
update(gMsg);

Here the update ist still inside of the lock. And the lock is even hold over
several dequeues. By this you prevent enqueueing, till the consumer has
emptied the queue.

Christof

 

[cty0000]

What he meant is, change youre original threadJob:

public void threadJob()
{
GUIMsg gMsg;

while (true)
{
Thread.Sleep(100);
lock (queueLock)
{
while (GuiMsgQueue.Count == 0)
Monitor.Wait(queueLock);

gMsg = GuiMsgQueue.Dequeue();
}
update(gMsg); //This line is moved.
}
}

After the GuiMsgQueue.Dequeue() there is no need to hold the lock.

From your code:



Here the update ist still inside of the lock. And the lock is even hold over
several dequeues. By this you prevent enqueueing, till the consumer has
emptied the queue.

Christof




- -

Thanks you guys... I was stupid ^^...
I could pull out update like follow, the code work well Thanks.. a
loooooooooooooooooot

public void addNewMsg(GUIMsg msg)
{

lock (queueLock)
{
GuiMsgQueue.Enqueue(msg);
}

Thread guiUpdateThread = new Thread(new
ThreadStart(threadJob));
guiUpdateThread.Start();


}

private void threadJob()
{
GUIMsg gMsg;


while (GuiMsgQueue.Count != 0)
{
lock (queueLock)
{
gMsg = GuiMsgQueue.Dequeue();
}
update(gMsg);
}