Re: Synchronization exception
- From: "Peter Duniho" <NpOeStPeAdM@xxxxxxxxxxxxxxxx>
- Date: Mon, 05 Jan 2009 14:26:49 -0700
On Mon, 05 Jan 2009 14:01:01 -0700, David <DavidLBlackDeck@xxxxxxxxxxxxxxxx> wrote:
[...]
What is an "unsynchronized" block of code?
That's the error you get if you try call a method that requires the calling thread to own the synchronization object when the calling thread doesn't actually own the synchronization object.
Calling Monitor.Exit() when you haven't called Monitor.Enter() would be an example of that, but there are other ways to get the exception too.
I'm confident that you can't hit
the Exit method without having hit the Enter method two lines before,
Certainly the code you posted doesn't appear to be able to in and of itself generate the exception you describe. It has some bugs (a synchronization bug in the setter, and both the getter and setter may return without releasing the lock, if an exception were to occur...the latter bug being more an issue in the setter where you call to another method). But I don't think those would cause the exception you're seeing.
The problem is, you posted only a tiny amount of the actual code. Without a concise-but-complete code sample that reliably reproduces the problem, there's no way for anyone else to know what you've done wrong. We can only tell you that you have in fact done something wrong.
so the
lock owner really should be the thread in question every time. The Help file
says that the thread calling Exit() must be the owner of the lock. Is the
"owner" of the lock the thread that called Enter()? That's what I assume,
but it's obvious that in this case, you can only get to the ExitI() method
after calling the Enter() method, so doesn't that make that thread the owner?
All of that is true, which means that you've left out some important piece of information. After all, you _are_ getting the exception.
Please create a concise-but-complete code sample that reliably reproduces the problem, and I or someone else will gladly look at it and try to determine what's wrong with the code.
In the meantime, here's an example of a more elegant way to implement the property as you've posted it:
private object objLock = new object();
private CycleStates vCurrentCycleState;
public CycleStates CurrentCycleState
{
get
{
lock (objLock)
{
return vCurrentCycleState;
}
}
set
{
lock (objLock)
{
if (value != vCurrentCycleState)
{
switch (value)
{
case CycleStates.ActiveState:
Activate();
break;
case CycleStates.InactiveState:
DeActivate();
break;
}
vCurrentCycleState = value;
}
}
}
}
The C# "lock" statement is basically a way to use Monitor.Enter()/Exit() in a safe, concise way. In particular, it is not possible to exit the block of code protected by the "lock" statement without releasing the lock (it essentially uses a try/finally around the block).
I changed the setter so that the code gets the lock before inspecting the variable of interest. This ensures that another thread trying to set the value at the same time won't result in erroneous duplicated activate/deactivate calls, as well as simply ensuring that the variable is actually set as expected.
I also introduced a new variable for locking. It is generally a better idea to not use the object being synchronized as the actual synchronization reference, because if you do use the object as the synchronization reference, that opens the possibility of some other code also using it as a synchronization reference without your knowledge. At best, this could cause unintended contention for the lock, and at worst, it could result in a difficult-to-debug deadlock scenario.
One thing I left along was the order in which you update the variable relative to calling the activate/deactivate methods. You should be aware that in all of the .NET classes I can think of off the top of my head, a property is updated _before_ calling a method that signals the update, so that when that method is called, the new value is available. The inconsistency between your code and the .NET convention could lead to maintenance issues, so you should be very sure this is really how you want/need it to be.
Finally, I simply assigned the property value to the variable in the setter; in your original code, you assign a value other than the one passed in, which would result in your code getting repeated calls to Activate every time you try to assign CycleStates.ActiveState to the property (since the new value wouldn't wind up being the one being assigned, your test to see if the current value is the same as being assigned would always fail).
Without knowing anything about the rest of the code, I can't say for sure that the last change described above is actually correct. However, if it's not correct in your eyes, then I would strongly suggest your broader design may be flawed, as the behavior implemented in your original code certainly is unusual and counter-intuitive (a situation that leads to hard-to-maintain code).
Pete
.
- References:
- Synchronization exception
- From: David
- Synchronization exception
- Prev by Date: Re: Synchronization exception
- Next by Date: Re: Synchronization exception
- Previous by thread: Re: Synchronization exception
- Next by thread: Issue when running ADPREP
- Index(es):
Relevant Pages
|
