May 30, 2012
On Wed, 30 May 2012 16:46:54 +0100, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 5/30/12 2:34 AM, deadalnix wrote:
>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>> I would say that breaking things here, with the right deprecation
>>>> process, is the way to go.
>>>
>>> So what should we use for mutex-based synchronization if we deprecate
>>> synchronized classes?
>>>
>>> Andrei
>>
>> I think something similar to range design here is the way to go.
>>
>> It is easy to define something like
>>
>> template isLockable(T) {
>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>> is(typeof(T.init.release()));
>> }
>>
>> And allow locking only if(isLockable!Type) .
>>
>> Now we can create SyncObject or any structure we want. The point is that
>> we lock explicit stuff.
>
> But in this design anyone can lock such an object, which was something you advocated against.

I think there is some confusion here as to what the "problem" is and is not.

The problem is /not/ that you can lock any object.
The problem is /not/ that we have synchronized(object) {}
The problem is /not/ that we have synchronized classes/methods.

The problem /is/ that synchronized classes/methods use a mutex which is exposed publicly, and it the same mutex as used by synchronized(object) {}.  This exposure/re-use makes deadlocks more likely to happen, and harder to spot.

Removal of this "problem" will not stop deadlocks from happening as they're a problem multi-threaded/lock based code will always have (*).  It will however make them far less likely to happen accidentally.  It will make programmers think about what/where/when and how to lock things rather than blithely using synchronized everywhere.  It will mean using locks is not as easy as currently, though I think we can make it fairly nice with good library code and/or some co-operation between the runtime and synchronized() statements i.e. requiring the class implement a common Lockable interface for example.

The fact that every object can be locked, and this means they all use more memory is a side issue - arguably as important for some.

R

(*) the best you can do to "solve" the deadlock problem is to impose lock acquisition ordering on your code, as in all locks must be acquired in a defined order, and if not an assertion/error/exception is thrown.  This requires some sort of static/singelton/overlord class which is aware of all mutexes and is involved in all acquisitions/releases.

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
May 30, 2012
On 5/30/12 6:10 AM, Regan Heath wrote:
> Ultimately the "problem" that needs solving is the fact that
> synchronized classes/methods use a public mutex and this exposure makes
> deadlocks more likely. I was hoping that by giving some examples in
> code, it would become clearer to the people who don't see what all the
> fuss is about.

How do free-floating mutexes that are not explicitly associated with _any_ data make deadlocks less likely? In my experience the less structured mutexes are worse, not better.

> Yes, having to manually create a mutex is a pain, and as Andrei has
> mentioned a few times having 2 separate things (one object to lock, one
> mutex) which conceptually should be one thing is worse from an
> understanding/maintenance point of view, however..
>
> 1. If the mutex is intended to lock a single object then we can make the
> following options available:
> a. A Lockable class to derive from.
> b. A Lockable struct to compose with (assuming here that the struct will
> be created/destroyed with the object).
> c. A Lockable!(T) wrapper template/struct/class to wrap objects to be
> locked.
> d. A Lockable interface which classes may implement.
> e. Some sort of compiler magic where it supplies the mutex for
> objects/methods marked with "synchronized" or involved in a
> synchronized(object) statement.
>
> 2. If the scope of the locking is greater than a single object, then a
> separate mutex is required/desired anyway.

But all of these are available in the current design. You have synchronized classes for the likes of 1a and the core mutex for creating all others except e.

> 1a. would contain a mutex primitive and lock/tryLock/unlock methods (it
> would implement the interface in 1d)
>
> 1b. would also contain a mutex primitive and alias could be used to
> expose the lock/tryLock/unlock methods (it would implement the interface
> in 1d)
>
> 1c. is actually a technique I have used a lot in C++ where the template
> class is created on the stack of a function/critical section and it
> (optionally) locks on creation (or later when lock() is called) and
> always unlocks on destruction as it leaves scope (by any means). It's a
> very robust pattern, and is similar (the inspiration/or result) of/to
> the synchronized block itself.

Yah, we use it a lot at Facebook. We probably should have such a wrapper in the standard library.

> IMO, the wasted space used by the monitor isn't ideal, but it's not a
> "problem" for the general case. It is a problem for people with limited
> memory environments but that's another topic. Talking about both is only
> clouding the discussional waters so to speak.

I agree.


Andrei

May 30, 2012
On 5/30/12 6:39 AM, deadalnix wrote:
> Le 30/05/2012 14:43, Regan Heath a écrit :
>> Not in all cases. If you have a synchronized class, say a thread-safe
>> container, which has a synchronized lookup and synchronized insert
>> function, code like..
>>
>> if (!o.lookup(...))
>> o.insert(...)
>>
> The correct solution is an insertIfNotPresent function, or to
> throw/return an error when inserting an item.
>
> Exposing the lock is just spreading the mess.

Agreed on both counts.


Andrei
May 30, 2012
On 30-05-2012 15:10, Regan Heath wrote:
> On Wed, 30 May 2012 13:43:14 +0100, deadalnix <deadalnix@gmail.com> wrote:
>> Le 30/05/2012 14:32, Regan Heath a écrit :
>>> 1. Prevent locking on any/every object. People would have to create a
>>> separate mutex object for locking. This is a little tedious, but it does
>>> make people think about the scope of the locking (where to put the
>>> mutex, who can see/use it, etc). On the flipside it can make people
>>> re-use a mutex for multiple conceptual critical section areas of code,
>>> which is less than ideal, but at least it's not 'broken'.
>>>
>>
>> It is suboptimal, but correct.
>
> Indeed, and can be made optimal with some fine-tuning/work.
>
>>> 2. Allow locking on any/every object but use a different mutex for
>>> synchronized class/methods. So 'synchronized' on a method call locks a
>>> private mutex object, not the object itself. And synchronized(object)
>>> locks the public mutex object. The downside here is that now every
>>> object potentially has 2 mutex objects/handles internally - a public and
>>> a private one.
>>>
>>
>> It doesn't address most of the drawback cited. Notably the fact that
>> every object have a monitor field, but most of them will not use it.
>
> True, I was just mentioning it as an option that solves the key
> issue/problem - not as the best solution to that problem. I think #1 is
> better.
>
> Ultimately the "problem" that needs solving is the fact that
> synchronized classes/methods use a public mutex and this exposure makes
> deadlocks more likely. I was hoping that by giving some examples in
> code, it would become clearer to the people who don't see what all the
> fuss is about.
>
> Yes, having to manually create a mutex is a pain, and as Andrei has
> mentioned a few times having 2 separate things (one object to lock, one
> mutex) which conceptually should be one thing is worse from an
> understanding/maintenance point of view, however..
>
> 1. If the mutex is intended to lock a single object then we can make the
> following options available:
> a. A Lockable class to derive from.
> b. A Lockable struct to compose with (assuming here that the struct will
> be created/destroyed with the object).
> c. A Lockable!(T) wrapper template/struct/class to wrap objects to be
> locked.
> d. A Lockable interface which classes may implement.
> e. Some sort of compiler magic where it supplies the mutex for
> objects/methods marked with "synchronized" or involved in a
> synchronized(object) statement.
>
> 2. If the scope of the locking is greater than a single object, then a
> separate mutex is required/desired anyway.
>
> 1a. would contain a mutex primitive and lock/tryLock/unlock methods (it
> would implement the interface in 1d)
>
> 1b. would also contain a mutex primitive and alias could be used to
> expose the lock/tryLock/unlock methods (it would implement the interface
> in 1d)
>
> 1c. is actually a technique I have used a lot in C++ where the template
> class is created on the stack of a function/critical section and it
> (optionally) locks on creation (or later when lock() is called) and
> always unlocks on destruction as it leaves scope (by any means). It's a
> very robust pattern, and is similar (the inspiration/or result) of/to
> the synchronized block itself.

In D, you can kind of already do that:

void foo()
{
    mtx.lock();
    scope (exit) mtx.unlock();

    // ...
}

It does involve that one extra scope (exit) line, but I'm of the opinion that being explicit about that is a good thing.

>
> 1d. could be a requirement for classes which are to be used in
> synchronized statements i.e.
>
> class A : Lockable
> {
> void lock() {}
> void unlock() {}
> }
>
> A a = new A();
> synchronized(a)
> { // calls a.lock()
> ...
> } // calls a.unlock()
>
> 1e. I leave as a opener to Walter/Andrei/the reader .. I've seen many a
> genius idea pop out of nowhere on this forum.
>
> IMO, the wasted space used by the monitor isn't ideal, but it's not a
> "problem" for the general case. It is a problem for people with limited
> memory environments but that's another topic. Talking about both is only
> clouding the discussional waters so to speak.
>
> R
>


-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
May 30, 2012
On 5/30/12 9:03 AM, Regan Heath wrote:
> On Wed, 30 May 2012 16:46:54 +0100, Andrei Alexandrescu
> <SeeWebsiteForEmail@erdani.org> wrote:
>
>> On 5/30/12 2:34 AM, deadalnix wrote:
>>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>>> I would say that breaking things here, with the right deprecation
>>>>> process, is the way to go.
>>>>
>>>> So what should we use for mutex-based synchronization if we deprecate
>>>> synchronized classes?
>>>>
>>>> Andrei
>>>
>>> I think something similar to range design here is the way to go.
>>>
>>> It is easy to define something like
>>>
>>> template isLockable(T) {
>>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>>> is(typeof(T.init.release()));
>>> }
>>>
>>> And allow locking only if(isLockable!Type) .
>>>
>>> Now we can create SyncObject or any structure we want. The point is that
>>> we lock explicit stuff.
>>
>> But in this design anyone can lock such an object, which was something
>> you advocated against.
>
> I think there is some confusion here as to what the "problem" is and is
> not.
>
> The problem is /not/ that you can lock any object.
> The problem is /not/ that we have synchronized(object) {}
> The problem is /not/ that we have synchronized classes/methods.

Several posts in this thread assert that such are problems.

> The problem /is/ that synchronized classes/methods use a mutex which is
> exposed publicly, and it the same mutex as used by synchronized(object)
> {}. This exposure/re-use makes deadlocks more likely to happen, and
> harder to spot.

This is news to me. How do you publicly access the mutex of a synchronized class object?


Andrei

May 30, 2012
On 30-05-2012 18:03, Regan Heath wrote:
> On Wed, 30 May 2012 16:46:54 +0100, Andrei Alexandrescu
> <SeeWebsiteForEmail@erdani.org> wrote:
>
>> On 5/30/12 2:34 AM, deadalnix wrote:
>>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>>> I would say that breaking things here, with the right deprecation
>>>>> process, is the way to go.
>>>>
>>>> So what should we use for mutex-based synchronization if we deprecate
>>>> synchronized classes?
>>>>
>>>> Andrei
>>>
>>> I think something similar to range design here is the way to go.
>>>
>>> It is easy to define something like
>>>
>>> template isLockable(T) {
>>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>>> is(typeof(T.init.release()));
>>> }
>>>
>>> And allow locking only if(isLockable!Type) .
>>>
>>> Now we can create SyncObject or any structure we want. The point is that
>>> we lock explicit stuff.
>>
>> But in this design anyone can lock such an object, which was something
>> you advocated against.
>
> I think there is some confusion here as to what the "problem" is and is
> not.
>
> The problem is /not/ that you can lock any object.
> The problem is /not/ that we have synchronized(object) {}
> The problem is /not/ that we have synchronized classes/methods.
>
> The problem /is/ that synchronized classes/methods use a mutex which is
> exposed publicly, and it the same mutex as used by synchronized(object)
> {}. This exposure/re-use makes deadlocks more likely to happen, and
> harder to spot.

Thanks. I think the discussion was getting a bit derailed, myself being guilty of going off on tangential issues. Yes, the issue is exactly what you point out here: Exposing a locking resource publicly. The issue with a monitor on every object can probably be solved completely separately.

>
> Removal of this "problem" will not stop deadlocks from happening as
> they're a problem multi-threaded/lock based code will always have (*).
> It will however make them far less likely to happen accidentally. It
> will make programmers think about what/where/when and how to lock things
> rather than blithely using synchronized everywhere. It will mean using
> locks is not as easy as currently, though I think we can make it fairly
> nice with good library code and/or some co-operation between the runtime
> and synchronized() statements i.e. requiring the class implement a
> common Lockable interface for example.

One gripe I have with using interfaces is speed. David Simcha showed that virtual mutex calls actually *do* impact speed significantly, particularly in the GC in his case.

As I pointed out earlier in this thread, core.sync.mutex.Mutex allows both composition and inheritance, and works with the synchronized statement. I still think it satisfies what everyone wants, given those two facts, and I have yet to see any counter-arguments to that (but would love to hear some to get a better understanding of why some people are against mutexes).

>
> The fact that every object can be locked, and this means they all use
> more memory is a side issue - arguably as important for some.
>
> R
>
> (*) the best you can do to "solve" the deadlock problem is to impose
> lock acquisition ordering on your code, as in all locks must be acquired
> in a defined order, and if not an assertion/error/exception is thrown.
> This requires some sort of static/singelton/overlord class which is
> aware of all mutexes and is involved in all acquisitions/releases.
>

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
May 30, 2012
On 30-05-2012 18:14, Andrei Alexandrescu wrote:
> On 5/30/12 9:03 AM, Regan Heath wrote:
>> On Wed, 30 May 2012 16:46:54 +0100, Andrei Alexandrescu
>> <SeeWebsiteForEmail@erdani.org> wrote:
>>
>>> On 5/30/12 2:34 AM, deadalnix wrote:
>>>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>>>> I would say that breaking things here, with the right deprecation
>>>>>> process, is the way to go.
>>>>>
>>>>> So what should we use for mutex-based synchronization if we deprecate
>>>>> synchronized classes?
>>>>>
>>>>> Andrei
>>>>
>>>> I think something similar to range design here is the way to go.
>>>>
>>>> It is easy to define something like
>>>>
>>>> template isLockable(T) {
>>>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>>>> is(typeof(T.init.release()));
>>>> }
>>>>
>>>> And allow locking only if(isLockable!Type) .
>>>>
>>>> Now we can create SyncObject or any structure we want. The point is
>>>> that
>>>> we lock explicit stuff.
>>>
>>> But in this design anyone can lock such an object, which was something
>>> you advocated against.
>>
>> I think there is some confusion here as to what the "problem" is and is
>> not.
>>
>> The problem is /not/ that you can lock any object.
>> The problem is /not/ that we have synchronized(object) {}
>> The problem is /not/ that we have synchronized classes/methods.
>
> Several posts in this thread assert that such are problems.
>
>> The problem /is/ that synchronized classes/methods use a mutex which is
>> exposed publicly, and it the same mutex as used by synchronized(object)
>> {}. This exposure/re-use makes deadlocks more likely to happen, and
>> harder to spot.
>
> This is news to me. How do you publicly access the mutex of a
> synchronized class object?

Generally in two ways:

1) synchronized (obj)
2) obj.__monitor

(1) accesses it in order to actually do locking, but if obj is an instantiation of a class that uses the this reference internally for locking, you end up with potential deadlocks. Therefore, you can say that obj's mutex is exposed. This isn't necessarily bad, because if obj does *not* use its this reference for synchronized statements, nothing will blow up. This is why the OP was about banning synchronized (this).

(2) is an undocumented feature of the language that druntime (and maybe some phobos code) makes use of to create/alter/delete monitors.

>
>
> Andrei
>

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
May 30, 2012
On 30 May 2012 17:23, Alex Rønne Petersen <alex@lycus.org> wrote:
> On 30-05-2012 18:14, Andrei Alexandrescu wrote:
>>
>> On 5/30/12 9:03 AM, Regan Heath wrote:
>>>
>>> On Wed, 30 May 2012 16:46:54 +0100, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:
>>>
>>>> On 5/30/12 2:34 AM, deadalnix wrote:
>>>>>
>>>>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>>>>>
>>>>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>>>>>
>>>>>>> I would say that breaking things here, with the right deprecation process, is the way to go.
>>>>>>
>>>>>>
>>>>>> So what should we use for mutex-based synchronization if we deprecate synchronized classes?
>>>>>>
>>>>>> Andrei
>>>>>
>>>>>
>>>>> I think something similar to range design here is the way to go.
>>>>>
>>>>> It is easy to define something like
>>>>>
>>>>> template isLockable(T) {
>>>>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>>>>> is(typeof(T.init.release()));
>>>>> }
>>>>>
>>>>> And allow locking only if(isLockable!Type) .
>>>>>
>>>>> Now we can create SyncObject or any structure we want. The point is
>>>>> that
>>>>> we lock explicit stuff.
>>>>
>>>>
>>>> But in this design anyone can lock such an object, which was something you advocated against.
>>>
>>>
>>> I think there is some confusion here as to what the "problem" is and is not.
>>>
>>> The problem is /not/ that you can lock any object.
>>> The problem is /not/ that we have synchronized(object) {}
>>> The problem is /not/ that we have synchronized classes/methods.
>>
>>
>> Several posts in this thread assert that such are problems.
>>
>>> The problem /is/ that synchronized classes/methods use a mutex which is exposed publicly, and it the same mutex as used by synchronized(object) {}. This exposure/re-use makes deadlocks more likely to happen, and harder to spot.
>>
>>
>> This is news to me. How do you publicly access the mutex of a synchronized class object?
>
>
> Generally in two ways:
>
> 1) synchronized (obj)
> 2) obj.__monitor
>
> (1) accesses it in order to actually do locking, but if obj is an instantiation of a class that uses the this reference internally for locking, you end up with potential deadlocks. Therefore, you can say that obj's mutex is exposed. This isn't necessarily bad, because if obj does *not* use its this reference for synchronized statements, nothing will blow up. This is why the OP was about banning synchronized (this).
>
> (2) is an undocumented feature of the language that druntime (and maybe some
> phobos code) makes use of to create/alter/delete monitors.
>

I'm pretty certain the use of .__monitor is stricted only to:
object_.d - The frontend implementation which contains the underlying
functions that are invoked when you synchronise(obj).
monitor_.d - The backend implementation which contains the platform
separated bits.


Regards
-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';
May 30, 2012
On Wed, 30 May 2012 17:00:43 +0100, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 5/30/12 5:32 AM, Regan Heath wrote:
>> On Wed, 30 May 2012 10:21:00 +0100, deadalnix <deadalnix@gmail.com> wrote:
>>> You don't want to synchronize on ANY object. You want to synchronize
>>> on explicit mutexes.
>>
>> +1 .. this is the key point/issue.
>
> TDPL's design only allows for entire synchronized classes (not separate synchronized and unsynchronized methods), which pair mutexes with the data they protect. This is more restrictive than exposing mutexes, but in a good way. We use such a library artifact in C++ at Facebook all the time, to great success.

Can you call pass them to a synchronized statement?  i.e.

TDPLStyleSynchClass a = new TDPLStyleSynchClass();
synchronized(a) {
}

.. because, if you can, then you're exposing the mutex.  If you can't, how do you do the above with these classes?  By explicitly calling a lock() or tryLock() method?  (do they implement a known interface?)

> People shouldn't create designs that have synchronized classes referring to one another naively. Designing with mutexes (explicit or implicit) will always create the possibility of deadlock, so examples how that could happen are easy to come across.

True.  But in my Example 1 the classes could be "entire" synchronized classes, and they do not refer to each other naively.  Instead, two threads have shared access to them and in each case they explicitly acquire one mutex (via a synchronized() statement), but implicitly acquire another (by calling a method).  It's the implicit acquisition which makes the bug hard to see and easy to do accidentally.

> TDPL improves on deadlocks by introducing synchronized statements with more than one argument, see 13.15.

Is there anywhere I can see this online? (for free :p)

> The implicit mutexes used by synchronized classes are recursive.

:) why would you want anything else.

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/
May 30, 2012
Le 30/05/2012 17:46, Andrei Alexandrescu a écrit :
> On 5/30/12 2:34 AM, deadalnix wrote:
>> Le 29/05/2012 23:33, Andrei Alexandrescu a écrit :
>>> On 5/29/12 1:37 AM, deadalnix wrote:
>>>> I would say that breaking things here, with the right deprecation
>>>> process, is the way to go.
>>>
>>> So what should we use for mutex-based synchronization if we deprecate
>>> synchronized classes?
>>>
>>> Andrei
>>
>> I think something similar to range design here is the way to go.
>>
>> It is easy to define something like
>>
>> template isLockable(T) {
>> enum isLockable = isShared!T && is(typeof(T.init.lock())) &&
>> is(typeof(T.init.release()));
>> }
>>
>> And allow locking only if(isLockable!Type) .
>>
>> Now we can create SyncObject or any structure we want. The point is that
>> we lock explicit stuff.
>
> But in this design anyone can lock such an object, which was something
> you advocated against.
>
> Andrei

I was advocating on being able to lock ANY object, which is out of control.

Such an object is known to be lockable, and most object will not be. It will avoid liquid lock, because most thing will not be lockable.

Combined with encapsulation capabilities that D already have, exposing the lock to the entire worlds is easy. It will avoid unexpected lock from 3rd party code, which is a source of tedious deadlock.

It also open door for locking on struct, that is easily embeded in a class as value (so constructed and destructed with the class).

The fact that anyone or not can lock/unlock a mutex is a encapsulation problem and we already have solution in D for that.