synchronized (this[.classinfo]) in druntime and phobos

[Alex Rønne Petersen]

On 29-05-2012 14:07, Regan Heath wrote:
> On Tue, 29 May 2012 01:08:59 +0100, Jonathan M Davis
> <jmdavisProg at gmx.com> wrote:
>
>> On Tuesday, May 29, 2012 01:54:59 Alex Rønne Petersen wrote:
>>> On 29-05-2012 01:41, Jonathan M Davis wrote:
>>> > On Tuesday, May 29, 2012 01:35:23 Alex Rønne Petersen wrote:
>>> >> I don't think arguing about them makes sense at this point. Way
>>> too much
>>> >> code would break if we changed the semantics. I'd consider it a
>>> mistake
>>> >> and a lesson learned, rather.
>>> >>
>>> >> But I take it you agree that synchronized (this) and similar
>>> >> "uncontrollable" synchronization blocks should be avoided?
>>> >
>>> > I'm not an expert on threading stuf, but it would be my opinion
>>> that if
>>> > you're not intending to protect the entire class with locks that it
>>> makes
>>> > no sense to lock on the class itself. You're locking for something
>>> > specific, in which case, your sychronized block should be locking on
>>> > something else specific to what you're trying to protect. Certainly,
>>> > that's how I'd approach it with mutexes. You don't have a mutex for an
>>> > entire class unless it's actually used for all of the class'
>>> functions.
>>> > Rather, you use mutexes specific to what you're trying to protect.
>>> >
>>> > - Jonathan M Davis
>>>
>>> Right, but even if you really *are* protecting the entire class, you can
>>> still create mysterious deadlocks if users of your code lock on your
>>> class. So I'm arguing that no matter the use case, never lock on a
>>> 'this' reference exposed outside of some API layer.
>>
>> Well, it seems pretty abysmal to me to be locking on something that
>> you don't control. Making a mutex that your class used public would
>> just be stupid.
>
> Interestingly this is what C#s Array type does with SyncRoot
> (intentionally).

Microsoft has officially dismissed the SyncRoot approach to 
synchronization. I don't have a link handy, but there's an article 
*somewhere* on MSDN about it.

>
>> With synchronized classes/functions, you're basically creating and
>> using an
>> implicit mutex for the whole class, which would then be the same as if
>> you locked it at the beginning of every member function call and
>> unlocked it at its end, which doesn't expose the mutex at all. So, I
>> don't really see te
>> problem there would have to study the matter more to see what the
>> issue there is.
>
> According to the docs here:
> http://dlang.org/class.html#synchronized-functions
>
> A synchronized function locks "this" and as "this" is exposed
> publicly.... In the following code the "lock" statement and
> "synchronized void bar" lock the same mutex.
>
> class Foo {
> synchronized void bar() { ...statements... }
> }
>
> void main()
> {
> Foo foo = new Foo();
> lock(foo)
> {
> ...statements...
> }
> }
>
>> But locking on another class rather than something specifically
>> intended as a mutex does seem to me like it's asking for trouble.
>
> Yep. It commonly arises where you have a class/object which is either
> not synchronized itself (because it might be used in single threaded
> situations and you want performance) like a collection class for
> example, or is synchronized but you need to lock a sequence of member
> function calls i.e. a lookup and insert on the collection. What happens
> then, is people just call lock(<object>) and end up locking the same
> mutex as the class does internally. What happens next to cause a
> deadlock is that inside that lock they call one or more functions or
> methods which internally call methods on another synchronized object.
> This results in a locking pattern of object1, object2. In another piece
> of code, running in another thread, they do something similar which
> results in a locking pattern of object2, object1 and eventually these
> threads will deadlock each other.
>
> R
>

-- 
Alex Rønne Petersen
alex at lycus.org
http://lycus.org