On Tue, 29 May 2012 01:08:59 +0100, Jonathan M Davis <jmdavisp...@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).

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

--
Using Opera's revolutionary email client: http://www.opera.com/mail/

Reply via email to