Having trouble with multiple threads and a shared resource

G

Guest

Hey guys,

In my app I have a bitmap where drawing is done and in the form's paint
method I show the bitmap:

private void Form1_Paint(object sender, System.Windows.Forms.PaintEventArgs e)
{
if(MyBitmap != null)
{
Graphics g = e.Graphics;
g.ResetClip();
Rectangle r = new Rectangle(0, 0, MyBitmap.Width, MyBitmap.Height);
g.DrawImage(MyBitmap, r, 0, 0, MyBitmap.Width, MyBitmap.Height,
GraphicsUnit.Pixel);
}
}

I also have a context menu option to do some work on the bitmap
(getting/setting some pixels). When the menu item is clicked, I start the
work on a separate thread because it takes a while and doing it in on the GUI
thread woul force me to use DoEvents() in order for the form to respond to
user interactions.

private void _replayPathTaken_Click(object sender, System.EventArgs e)
{
Thread replayPathThread = new Thread(new ThreadStart(ReplayPathTaken));

replayPathThread.Name = "ReplayPathTaken" + _count.ToString();
replayPathThread.IsBackground = true;
replayPathThread.Start();
_count++;
}

Now, ReplayPathTaken gathers some information and then calls another method,
HighlightSelectedCells. HightlightSelectedCells does a lot of work on
MyBitmap's pixles, similar to the following:

pixelColor = MyBitmap.GetPixel(selectedCell.Col,selectedCell.Row);

if(pixelColor == highlightColor)
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.SolveColor);
}
else
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.BacktrackColor);
}

//Update screen with new bitmap
Invalidate(new Rectangle

(selectedCell.Col,selectedCell.Row,Cell.CellSize,Cell.CellSize));
Update();

So, the problem is that both the Form1_Paint method and
HighlightSelectedCells method need access to MyBitmap and sometimes they both
try to use it at the same time. That gives me the following error:

A first chance exception of type 'System.InvalidOperationException' occurred
in system.drawing.dll

Additional information: The object is currently in use elsewhere.


How can I make sure that only one thread has access to the bitmap at a time
and that my form remains responsive to user interaction (such as moving the
form or resizing it)?

Also, I would like to allow the user to click the menu item and start
_replayPathTaken_Click multiple times. This would create even more threads
that would need synchronized access to the bitmap.

If you have any questions please let me know. Any help is greatly
appreciated :)

-Flack
 
N

Nicholas Paldino [.NET/C# MVP]

Flack,

Have you looked at the lock statement? You could use it to synchronize
access between the threads to the bitmap. Assuming you have a valid
reference, you can use the same reference in the lock statement, like so:

lock (MyBitmap)
{
// Do work
}

Hope this helps.
 
G

Guest

Just a small addition. The error always seems to occur on either a GetPixel
call in HighlightSelectedCells or in Form1_Paint at line

Rectangle r = new Rectangle(0, 0, MazeBitmap.Width, MazeBitmap.Height);

Here is the error message:
System.InvalidOperationException: The object is currently in use elsewhere.
at System.Drawing.Bitmap.GetPixel(Int32 x, Int32 y)
at FloorsTest.Maze.HighlightSelectedCells(ArrayList selectedRows_,
ArrayList previouslySelectedRows_) in c:\...\:line 1864
at FloorsTest.Maze.ReplayPathTaken() in c:\...\:line 2033

Line 1864 is:
pixelColor = MyBitmap.GetPixel(selectedCell.Col,selectedCell.Row);

Line 2033 is:
HighlightSelectedCells(current,previous);

Also, MyBitmap is just a property that gets/sets the bitmap. It doesn't do
anyhting else.
 
G

Guest

Hey Nick,

Thanks for the tip. I'll give it a try. Still pretty inexperienced with
multi threading.
I'll let you know how it goes.


Nicholas Paldino said:
Flack,

Have you looked at the lock statement? You could use it to synchronize
access between the threads to the bitmap. Assuming you have a valid
reference, you can use the same reference in the lock statement, like so:

lock (MyBitmap)
{
// Do work
}

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Flack said:
Hey guys,

In my app I have a bitmap where drawing is done and in the form's paint
method I show the bitmap:

private void Form1_Paint(object sender,
System.Windows.Forms.PaintEventArgs e)
{
if(MyBitmap != null)
{
Graphics g = e.Graphics;
g.ResetClip();
Rectangle r = new Rectangle(0, 0, MyBitmap.Width, MyBitmap.Height);
g.DrawImage(MyBitmap, r, 0, 0, MyBitmap.Width, MyBitmap.Height,
GraphicsUnit.Pixel);
}
}

I also have a context menu option to do some work on the bitmap
(getting/setting some pixels). When the menu item is clicked, I start the
work on a separate thread because it takes a while and doing it in on the
GUI
thread woul force me to use DoEvents() in order for the form to respond to
user interactions.

private void _replayPathTaken_Click(object sender, System.EventArgs e)
{
Thread replayPathThread = new Thread(new ThreadStart(ReplayPathTaken));

replayPathThread.Name = "ReplayPathTaken" + _count.ToString();
replayPathThread.IsBackground = true;
replayPathThread.Start();
_count++;
}

Now, ReplayPathTaken gathers some information and then calls another
method,
HighlightSelectedCells. HightlightSelectedCells does a lot of work on
MyBitmap's pixles, similar to the following:

pixelColor = MyBitmap.GetPixel(selectedCell.Col,selectedCell.Row);

if(pixelColor == highlightColor)
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.SolveColor);
}
else
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.BacktrackColor);
}

//Update screen with new bitmap
Invalidate(new Rectangle

(selectedCell.Col,selectedCell.Row,Cell.CellSize,Cell.CellSize));
Update();

So, the problem is that both the Form1_Paint method and
HighlightSelectedCells method need access to MyBitmap and sometimes they
both
try to use it at the same time. That gives me the following error:

A first chance exception of type 'System.InvalidOperationException'
occurred
in system.drawing.dll

Additional information: The object is currently in use elsewhere.


How can I make sure that only one thread has access to the bitmap at a
time
and that my form remains responsive to user interaction (such as moving
the
form or resizing it)?

Also, I would like to allow the user to click the menu item and start
_replayPathTaken_Click multiple times. This would create even more
threads
that would need synchronized access to the bitmap.

If you have any questions please let me know. Any help is greatly
appreciated :)

-Flack
 
B

Brian Gideon

Flack,

Can you create a copy of the bitmap that the worker thread will use
before kicking it off in the _replayPathTaken_Click event? That way
the UI thread and worker thread are using separate instances of the
bitmap. Once the worker thread is done manipulating the bitmap inform
the UI thread that a new bitmap is available by calling Control.Invoke.
You wouldn't need to use locking mechanisms which would just hang up
the UI thread if the worker thread holds the lock.

Brian
 
G

Guest

Well, using lock(MyBitmap) seems to have done the trick. I put the lock
statements around any lines where the bitmap was going to be accessed (such
as Get/Set Pixel and in Form1_Paint). Thanks for the tip.

I have two other quick questions though.

1. I read that you should use a static private object as a lock, such as
private static object myLock = new object();
Is there any difference between using that and using MyBitmap as the lock?

2. Lets say I have a line like

Graphics gScreen = CreateGraphics();

in my HightlightSelectedCells method (which runs in its own thread). Now,
when I close the form, I get an error:

A first chance exception of type 'System.ObjectDisposedException' occurred
in system.windows.forms.dll
Additional information: Cannot access a disposed object named "Maze"

because the form has been disposed. I thought Background threads are ended
before the form is disposed so this situation wouldn't arise. Is my only
option to have a check, like if "form not disposed" continue?

Thanks for the help.

Nicholas Paldino said:
Flack,

Have you looked at the lock statement? You could use it to synchronize
access between the threads to the bitmap. Assuming you have a valid
reference, you can use the same reference in the lock statement, like so:

lock (MyBitmap)
{
// Do work
}

Hope this helps.


--
- Nicholas Paldino [.NET/C# MVP]
- (e-mail address removed)

Flack said:
Hey guys,

In my app I have a bitmap where drawing is done and in the form's paint
method I show the bitmap:

private void Form1_Paint(object sender,
System.Windows.Forms.PaintEventArgs e)
{
if(MyBitmap != null)
{
Graphics g = e.Graphics;
g.ResetClip();
Rectangle r = new Rectangle(0, 0, MyBitmap.Width, MyBitmap.Height);
g.DrawImage(MyBitmap, r, 0, 0, MyBitmap.Width, MyBitmap.Height,
GraphicsUnit.Pixel);
}
}

I also have a context menu option to do some work on the bitmap
(getting/setting some pixels). When the menu item is clicked, I start the
work on a separate thread because it takes a while and doing it in on the
GUI
thread woul force me to use DoEvents() in order for the form to respond to
user interactions.

private void _replayPathTaken_Click(object sender, System.EventArgs e)
{
Thread replayPathThread = new Thread(new ThreadStart(ReplayPathTaken));

replayPathThread.Name = "ReplayPathTaken" + _count.ToString();
replayPathThread.IsBackground = true;
replayPathThread.Start();
_count++;
}

Now, ReplayPathTaken gathers some information and then calls another
method,
HighlightSelectedCells. HightlightSelectedCells does a lot of work on
MyBitmap's pixles, similar to the following:

pixelColor = MyBitmap.GetPixel(selectedCell.Col,selectedCell.Row);

if(pixelColor == highlightColor)
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.SolveColor);
}
else
{
MyBitmap.SetPixel
(selectedCell.Col,selectedCell.Row
,_optionsProperties.BacktrackColor);
}

//Update screen with new bitmap
Invalidate(new Rectangle

(selectedCell.Col,selectedCell.Row,Cell.CellSize,Cell.CellSize));
Update();

So, the problem is that both the Form1_Paint method and
HighlightSelectedCells method need access to MyBitmap and sometimes they
both
try to use it at the same time. That gives me the following error:

A first chance exception of type 'System.InvalidOperationException'
occurred
in system.drawing.dll

Additional information: The object is currently in use elsewhere.


How can I make sure that only one thread has access to the bitmap at a
time
and that my form remains responsive to user interaction (such as moving
the
form or resizing it)?

Also, I would like to allow the user to click the menu item and start
_replayPathTaken_Click multiple times. This would create even more
threads
that would need synchronized access to the bitmap.

If you have any questions please let me know. Any help is greatly
appreciated :)

-Flack
 
M

Mehdi

1. I read that you should use a static private object as a lock, such as
private static object myLock = new object();
Is there any difference between using that and using MyBitmap as the lock?

If you are not familiar with multi-threading, then you really should read
Jon Skeet's excellent articles here:
http://www.yoda.arachsys.com/csharp/threads/index.shtml

And of particular relevance for you:
http://www.yoda.arachsys.com/csharp/threads/locking.shtml
http://www.yoda.arachsys.com/csharp/threads/lockchoice.shtml

It might take you a while to go through all that but tackling
multi-threading without a very good understanding of what's going on is a
perfect recipe for disaster.

As for choosing what to lock on, i don't know where you've read that you
should lock on a static object but this seems like a very bad idea to me in
most cases (unless the shared data is itself static of course). The main
rule is that all the methods that access your shared data must lock on the
same instance of your lock object (if they lock on different instances,
then the lock will obviously have no effect). In your case, provided that
MyBitmap always points to the same instance, it makes sense to lock on it.
However, i usually prefer to create a readonly object for the sole purpose
of locking on it:
private readonly object MyBitmapLock = new object();
....
lock (MyBitmapLock)
{
// Do whatever you want on MyBitmap here
}

The readonly ensures that MyBitmapLock will always point to the same
instance and that you will not inadvertently reaffect it to another
instance (which could mean the end of the world).
2. Lets say I have a line like

Graphics gScreen = CreateGraphics();

in my HightlightSelectedCells method (which runs in its own thread). Now,
when I close the form, I get an error:

A first chance exception of type 'System.ObjectDisposedException' occurred
in system.windows.forms.dll
Additional information: Cannot access a disposed object named "Maze"

I do not see any "Maze" in your code above. The error must appear somewhere
else.
because the form has been disposed. I thought Background threads are ended
before the form is disposed so this situation wouldn't arise. Is my only
option to have a check, like if "form not disposed" continue?

No, background threads are stopped when the application exits but there is
no garantee that they'll be stopped before UI controls are disposed.
Beware: you should not access UI controls directly from a worker thread. It
may be that you have more problems in your code than what you've told us so
far.

On another topic: if you are doing a lot of GetPixel and SetPixel calls in
a row, then you should really consider accessing your Bitmap data directly
using the LockBits method of the Bitmap class rather than using GetPixel
and SetPixel. It's a lot faster. In a basic test application i've made,
setting every pixel of a 1024x768 Bitmap took almost 1 second with the
SetPixel method but was virtually instant with the LockBits + direct data
access method. Bob Powell has a nice article about that here:
http://www.bobpowell.net/lockingbits.htm
 
M

Mehdi

I do not see any "Maze" in your code above. The error must appear somewhere
else.

Oops, i've just realized that this code probably is part of your Maze form
and that CreateGraphics() must be the one of the Maze Form method. In this
case, your code is thread safe as CreateGraphics() is one of the few
Control's method that you can call from a worker thread. So the answer of
your question is: yes you must check that your control is not disposed
before calling its CreateGraphics() method. The problem is that you simply
can not check wether a control is disposed or not from a worker thread.
E.g. this code:

if (!this.IsDisposed)
{
Graphics gScreen = CreateGraphics();
}

will fail from time to time since it is perfectly possible that your Maze
object will be disposed after you've checked for IsDisposed but before
you've have time to call CreateGraphics(). These are the wonders of
multi-threading.

Can't you create your Graphics object from your Bitmap inside your lock
statement instead of creating it from your form? This would solve this
particular problem.
 
G

Guest

Thanks for the replies Mehdi. They were really useful. I'll make sure to look
into the articles on locks and multithreading that you posted.

As for the CreateGraphics() call, you are correct in that it is a Maze form
method. It turns out that I do not need a Graphics object to be created from
the form after all (all I need is the Graphics object from the bitmap, which
I can safely create).

Also, I'll check out LockBits. Anything to make the bitmap manipulation
faster :)

Thanks a lot.
 

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