Deadlock between MethodImpl(MethodImplOptions.Synchronized) and lock(this) ?

  • Thread starter Thread starter Joannes Vermorel
  • Start date Start date
J

Joannes Vermorel

I am currently dealing with a multithreaded application. Basically, I have
the following class:

class MyClass {

public void Foo1() {
// do something (no method call)
lock(this) { /* do something (no method call) */ }
}

[MethodImpl(MethodImplOptions.Synchronized)]
public void Foo2() {
// do something (no method call)
}
}

And with 2 threads, I manage to get a deadlock between calls to Foo1() and
Foo2(). My guess (but I may be wrong) is that somehow MethodImpl is also
taking a lock on the MyClass instance. A quick view at MSDN did not tell if
such guess was true or not.

Can anyone confirm or refute the guess ?

Thanks,
Joannes
 
Joannes Vermorel said:
I am currently dealing with a multithreaded application. Basically, I have
the following class:

class MyClass {

public void Foo1() {
// do something (no method call)
lock(this) { /* do something (no method call) */ }
}

[MethodImpl(MethodImplOptions.Synchronized)]
public void Foo2() {
// do something (no method call)
}
}

And with 2 threads, I manage to get a deadlock between calls to Foo1() and
Foo2(). My guess (but I may be wrong) is that somehow MethodImpl is also
taking a lock on the MyClass instance. A quick view at MSDN did not tell if
such guess was true or not.

Can anyone confirm or refute the guess ?
Click to expand...

While I don't know for absolutely sure, I'd say that's almost certainly
the case.

Personally, I'd avoid MethodImplOptions.Synchronized entirely, *and*
avoid locking on "this" - lock on a reference only available with your
class. For instance:

object dataLock = new object();

public void Foo1()
{
lock (dataLock)
{
....
}
}
 
Personally, I'd avoid MethodImplOptions.Synchronized entirely, *and*
avoid locking on "this" - lock on a reference only available with your
class. For instance:

object dataLock = new object();

public void Foo1()
{
lock (dataLock)
{
....
}
}
Click to expand...

Thanks for the answer.

Also that's the second time you provide me an example with "object dataLock
= new object();". Until now, when I was implementing my classes, I was using
some random fields of the class for synchronization purposes. But I found
out that the use of <c>object</c>s that have no other purposes but to serve
for synchronization make IMO the code much more readable. For the cases
where memory is not a bottleneck, I found this quite an elegant practice. I
have no idea if it's commonly done this way though.

Joannes
 
Joannes Vermorel said:
Thanks for the answer.

Also that's the second time you provide me an example with "object dataLock
= new object();". Until now, when I was implementing my classes, I was using
some random fields of the class for synchronization purposes. But I found
out that the use of <c>object</c>s that have no other purposes but to serve
for synchronization make IMO the code much more readable. For the cases
where memory is not a bottleneck, I found this quite an elegant practice. I
have no idea if it's commonly done this way though.
Click to expand...

I think it is among those who care about multi-threading and
readability.

The way I see it, it has various benefits:

o The lock serving one purpose and one purpose only means it can be
clearly documented.

o Being an object which only the declaring class has access to means
that unexpected deadlocks due to other classes locking on it don't
occur.

I should write this up in an article some time...
 
Back
Top