Hello!

According to the docs it says that lock (this) is a problem if the instance
can be accessed publicly.
Can somebody explain what can happen if I have a lock section and use this
as the lock object like this.
lock(this)
{

}

//Tony

On 2010-06-20 17:09, Tony Johansson wrote:
> According to the docs it says that lock (this) is a problem if the instance
> can be accessed publicly.
> Can somebody explain what can happen if I have a lock section and use this
> as the lock object like this.
> lock(this)
> {
>
> }
>
http://haacked.com/archive/2006/08/08/threadingneverlockthisredux.aspx

--
J.

On 20-06-2010 11:09, Tony Johansson wrote:
> According to the docs it says that lock (this) is a problem if the instance
> can be accessed publicly.
> Can somebody explain what can happen if I have a lock section and use this
> as the lock object like this.
> lock(this)
> {
>
> }

You can do it.

If someone else locks on the object you can risk a deadlock.

lock(this) has traditionally been frowned on in the .NET world
for that reason.

It is some theoretical bullshit.

Obviously you need to think about what you lock on, but
it is quite general in multithreaded programming that you
need to have the brain "on" not "off".

Arne

On 20-06-2010 11:49, Jeroen Mostert wrote:
> On 2010-06-20 17:09, Tony Johansson wrote:
>> According to the docs it says that lock (this) is a problem if the
>> instance
>> can be accessed publicly.
>> Can somebody explain what can happen if I have a lock section and use
>> this
>> as the lock object like this.
>> lock(this)
>> {
>>
>> }
>>
> http://haacked.com/archive/2006/08/08/threadingneverlockthisredux.aspx

Quote:

"A while ago I wrote that you should never lock a value type ..."

I think that gives a good indication of the technical level
of the advice.

Something like on a scale from 1 to 10 around -2.

:-)

Arne

Arne Vajhøj wrote:
> [...]
>> http://haacked.com/archive/2006/08/08/threadingneverlockthisredux.aspx
>
> Quote:
>
> "A while ago I wrote that you should never lock a value type ..."
>
> I think that gives a good indication of the technical level
> of the advice. [...]

Perhaps you could be more specific. The advice to never lock on a value
type is actually very good advice, and for the reason the author states
(value types are boxed, so you never get the same instance twice when
using "lock" or other Monitor methods).

Or put another way, how high could the quality of advice be when given
by someone who thinks locking on a value type is actually a good idea?

Pete

Arne Vajhøj wrote:
> On 20-06-2010 11:09, Tony Johansson wrote:
>> According to the docs it says that lock (this) is a problem if the
>> instance
>> can be accessed publicly.
>> Can somebody explain what can happen if I have a lock section and use
>> this
>> as the lock object like this.
>> lock(this)
>> {
>>
>> }
>
> You can do it.
>
> If someone else locks on the object you can risk a deadlock.
>
> lock(this) has traditionally been frowned on in the .NET world
> for that reason.
>
> It is some theoretical bullshit.

It's only "theoretical bullshit" in the same sense that all "best
practices" advice in programming is "theoretical bullshit".

> Obviously you need to think about what you lock on, but
> it is quite general in multithreaded programming that you
> need to have the brain "on" not "off".

One could say the exact same thing about all of the various idioms,
rules-of-thumb, etc. we use in programming. Just the other day, you
(correctly, IMHO) argued in favor of making fields private. But as long
as someone is careful, what's _really_ the problem with that? None.
It's just "theoretical bullshit" that exposing fields in your class
could cause an issue.

The real problem is that it's not possible for any of us humans to be
careful 100% of the time. Our brains don't operate in a strictly "on"
or "off" state; instead, different people have different capacities for
avoiding mistakes, and even a given person has good days and bad days.

When one follows good advice, such as never locking on "this", it helps
avoid certain classes of bugs. Of course, there will always be other
bugs that one can introduce. But one might as well avoid the
easy-to-avoid ones.

It's unfortunate that the author of the article used the example they
did, because it's clearly contrived. Who calls Monitor.Enter() on an
object, and then just loses the reference to that object? That's silly.

But it's still good advice to keep your locking objects private, so that
one can ensure that when locking on an object, it's only ever being used
for a lock in code that one has control over. Concurrent programming is
hard enough without making it even more difficult to reason about the
state of your program.

So, don't make it more difficult to reason about the state of your
program. Encapsulate locking in exactly the same way you'd encapsulate
data in fields, and it will be much easier to avoid certain kinds of bugs.

Frankly, you do the entire community a great disservice when you call
good advice like that "bullshit".

Pete

On 20-06-2010 14:45, Peter Duniho wrote:
> Arne Vajhøj wrote:
>> [...]
>>> http://haacked.com/archive/2006/08/08/threadingneverlockthisredux.aspx
>>
>> Quote:
>>
>> "A while ago I wrote that you should never lock a value type ..."
>>
>> I think that gives a good indication of the technical level
>> of the advice. [...]
>
> Perhaps you could be more specific. The advice to never lock on a value
> type is actually very good advice,

Given that the C# compiler gives an error when attempting to
do it, then ...

Arne

On 20-06-2010 14:55, Peter Duniho wrote:
> Arne Vajhøj wrote:
>> On 20-06-2010 11:09, Tony Johansson wrote:
>>> According to the docs it says that lock (this) is a problem if the
>>> instance
>>> can be accessed publicly.
>>> Can somebody explain what can happen if I have a lock section and use
>>> this
>>> as the lock object like this.
>>> lock(this)
>>> {
>>>
>>> }
>>
>> You can do it.
>>
>> If someone else locks on the object you can risk a deadlock.
>>
>> lock(this) has traditionally been frowned on in the .NET world
>> for that reason.
>>
>> It is some theoretical bullshit.
>
> It's only "theoretical bullshit" in the same sense that all "best
> practices" advice in programming is "theoretical bullshit".

No.

It is theoretical bullshit because it is not a problem seen
in the real world.

There are a few other best practices that also fell in that
category, but most best practices are actually based
on solving real world problems.

>> Obviously you need to think about what you lock on, but
>> it is quite general in multithreaded programming that you
>> need to have the brain "on" not "off".
>
> One could say the exact same thing about all of the various idioms,
> rules-of-thumb, etc. we use in programming. Just the other day, you
> (correctly, IMHO) argued in favor of making fields private. But as long
> as someone is careful, what's _really_ the problem with that? None. It's
> just "theoretical bullshit" that exposing fields in your class could
> cause an issue.

Those two are not very comparable.

It is relative easy to find examples on how breaking encapsulation
by exposing implementation has created problems.

I have never seen or heard about a real world case where
the lock on this has actually created a problem.

Best practices are only true best practices if they solve
real problems.

Best practices that solves non-existing problems are
just crap being propagated blindly without anyone thinking
about it.

> The real problem is that it's not possible for any of us humans to be
> careful 100% of the time. Our brains don't operate in a strictly "on" or
> "off" state; instead, different people have different capacities for
> avoiding mistakes, and even a given person has good days and bad days.
>
> When one follows good advice, such as never locking on "this", it helps
> avoid certain classes of bugs. Of course, there will always be other
> bugs that one can introduce. But one might as well avoid the
> easy-to-avoid ones.

Please post a link to an actual case of the problem.

> But it's still good advice to keep your locking objects private, so that
> one can ensure that when locking on an object,

Unfortunately that is not possible in many of the most interesting
cases where you need locks on multiple instances.

> So, don't make it more difficult to reason about the state of your
> program. Encapsulate locking in exactly the same way you'd encapsulate
> data in fields, and it will be much easier to avoid certain kinds of bugs.

The ability to lock something is not always an internal thing
but something that the caller needs.

> Frankly, you do the entire community a great disservice when you call
> good advice like that "bullshit".

It is not good advice. It is just copy paste.

Arne

Arne Vajhøj wrote:
> On 20-06-2010 14:45, Peter Duniho wrote:
>> Arne Vajhøj wrote:
>>> [...]
>>>> http://haacked.com/archive/2006/08/08/threadingneverlockthisredux.aspx
>>>
>>> Quote:
>>>
>>> "A while ago I wrote that you should never lock a value type ..."
>>>
>>> I think that gives a good indication of the technical level
>>> of the advice. [...]
>>
>> Perhaps you could be more specific. The advice to never lock on a value
>> type is actually very good advice,
>
> Given that the C# compiler gives an error when attempting to
> do it, then ...

The "lock" statement is not the only way to lock (there is no error if
you use the Monitor class directly), and the compiler error even when
using the "lock" statement is easily bypassed simply by casting the
variable to object.

Do not underestimate the ability of people to circumvent the compiler's
attempts to keep you from doing some stupid. The advice to not lock a
value type is valid, whether or not the compiler tries to help you avoid
doing it.

Pete

Arne Vajhøj wrote:
> [...]
> I have never seen or heard about a real world case where
> the lock on this has actually created a problem.

You certainly have an over-confident sense of the value of your own
experiences as applying to every other programmer in existence.

The fact is, the question of using "this" for a lock is important enough
that the implementation for compiler-generated event accessors was fixed
in the C# 4.0 compiler so that it no longer locks on "this".

I'll take the experiences of those who are actually implementing .NET
and the C# compiler over yours any day when it comes to answering
questions about what's an important rule to follow or not when writing
..NET code.

Pete