On Wed, 30 May 2012 00:10:19 +0100, Jonathan M Davis <jmdavisProg@gmx.com> wrote:

> On Wednesday, May 30, 2012 01:02:53 Alex Rønne Petersen wrote:
>> But mutexes allow proper encapsulation by hiding the mutex resource. As
>> I've proven in the very OP of this thread, both druntime and phobos
>> suffer from the anti-pattern that is locking on a public resource when
>> you shouldn't. The language encourages doing it with this synchronized
>> business.
>
>> From your comments, it sounds to me like the problem is entirely with
> synchronized blocks, not sychronized classes. Synchronized classes don't have
> the issue of having externals lock on them without the explicit use of
> synchronized blocks. They're the equivalent of locking a mutex in every public
> function and releasing it afterwards. The problem is that that mutex is
> effectively public such that when a synchronized block is used on it, it locks
> on the same mutex. If no synchronized blocks are used, as far as I can tell,
> it's a non-issue.
>
> As such, wouldn't simply making the use of a sychronized block on a
> synchronized object illegal fix the problem?

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(...)

obtains and releases the mutex twice each, and in between another thread could call o.insert() and insert the missing object - resulting in 2 copies being inserted (or a duplicate insert exception being throw, or...).

Now, the above is bad design, but the only other choice is to expose lock() and unlock() methods (or the mutex - back to square 1one) to allow locking around multiple class, but now the caller has to be careful to call unlock in all code paths.  scope helps us in D, but this is actually the whole point of synchronized blocks - 1. ensuring the unlock always happens and 2. making the scope of the locking visible in the code.

R

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

On Wednesday, 30 May 2012 at 12:43:15 UTC, deadalnix wrote:
>
> 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.

That could be solved without abandoning object locks. If locking on all objects is allowed, objects in TLS could simply omit the monitor field. Locking would then be performed as:

if (object not in TLS) {
  insert expensive locking operation here
}
return

Yes, this would introduce an extra conditional jump whenever a lock is acquired/released, but the overhead is very small compared to the relatively expensive lock/unlock operation.

On 30.05.2012 16:43, Regan Heath wrote:
> On Wed, 30 May 2012 00:10:19 +0100, Jonathan M Davis
> <jmdavisProg@gmx.com> wrote:
>
>> On Wednesday, May 30, 2012 01:02:53 Alex Rønne Petersen wrote:
>>> But mutexes allow proper encapsulation by hiding the mutex resource. As
>>> I've proven in the very OP of this thread, both druntime and phobos
>>> suffer from the anti-pattern that is locking on a public resource when
>>> you shouldn't. The language encourages doing it with this synchronized
>>> business.
>>
>>> From your comments, it sounds to me like the problem is entirely with
>> synchronized blocks, not sychronized classes. Synchronized classes
>> don't have
>> the issue of having externals lock on them without the explicit use of
>> synchronized blocks. They're the equivalent of locking a mutex in
>> every public
>> function and releasing it afterwards. The problem is that that mutex is
>> effectively public such that when a synchronized block is used on it,
>> it locks
>> on the same mutex. If no synchronized blocks are used, as far as I can
>> tell,
>> it's a non-issue.
>>
>> As such, wouldn't simply making the use of a sychronized block on a
>> synchronized object illegal fix the problem?
>
> 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(...)
>
> obtains and releases the mutex twice each, and in between another thread
> could call o.insert() and insert the missing object - resulting in 2
> copies being inserted (or a duplicate insert exception being throw, or...).
>
> Now, the above is bad design, but the only other choice is to expose
> lock() and unlock() methods (or the mutex - back to square 1one) to
> allow locking around multiple class, but now the caller has to be
> careful to call unlock in all code paths. scope helps us in D, but this
> is actually the whole point of synchronized blocks - 1. ensuring the
> unlock always happens and 2. making the scope of the locking visible in
> the code.
>
synchronized class Container
{

void applyLocked(scope delegate void(Container _this));
...
}


-- 
Dmitry Olshansky

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.

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

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

On 30-05-2012 14:54, Thiez wrote:
> On Wednesday, 30 May 2012 at 12:43:15 UTC, deadalnix wrote:
>>
>> 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.
>
> That could be solved without abandoning object locks. If locking on all
> objects is allowed, objects in TLS could simply omit the monitor field.
> Locking would then be performed as:
>
> if (object not in TLS) {
> insert expensive locking operation here
> }
> return
>
> Yes, this would introduce an extra conditional jump whenever a lock is
> acquired/released, but the overhead is very small compared to the
> relatively expensive lock/unlock operation.

I don't understand how that would work. Where would you store the monitor reference?

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

On Wed, 30 May 2012 13:55:10 +0100, Dmitry Olshansky <dmitry.olsh@gmail.com> wrote:

> On 30.05.2012 16:43, Regan Heath wrote:
>> On Wed, 30 May 2012 00:10:19 +0100, Jonathan M Davis
>> <jmdavisProg@gmx.com> wrote:
>>
>>> On Wednesday, May 30, 2012 01:02:53 Alex Rønne Petersen wrote:
>>>> But mutexes allow proper encapsulation by hiding the mutex resource. As
>>>> I've proven in the very OP of this thread, both druntime and phobos
>>>> suffer from the anti-pattern that is locking on a public resource when
>>>> you shouldn't. The language encourages doing it with this synchronized
>>>> business.
>>>
>>>> From your comments, it sounds to me like the problem is entirely with
>>> synchronized blocks, not sychronized classes. Synchronized classes
>>> don't have
>>> the issue of having externals lock on them without the explicit use of
>>> synchronized blocks. They're the equivalent of locking a mutex in
>>> every public
>>> function and releasing it afterwards. The problem is that that mutex is
>>> effectively public such that when a synchronized block is used on it,
>>> it locks
>>> on the same mutex. If no synchronized blocks are used, as far as I can
>>> tell,
>>> it's a non-issue.
>>>
>>> As such, wouldn't simply making the use of a sychronized block on a
>>> synchronized object illegal fix the problem?
>>
>> 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(...)
>>
>> obtains and releases the mutex twice each, and in between another thread
>> could call o.insert() and insert the missing object - resulting in 2
>> copies being inserted (or a duplicate insert exception being throw, or...).
>>
>> Now, the above is bad design, but the only other choice is to expose
>> lock() and unlock() methods (or the mutex - back to square 1one) to
>> allow locking around multiple class, but now the caller has to be
>> careful to call unlock in all code paths. scope helps us in D, but this
>> is actually the whole point of synchronized blocks - 1. ensuring the
>> unlock always happens and 2. making the scope of the locking visible in
>> the code.
>>
> synchronized class Container
> {
>
> void applyLocked(scope delegate void(Container _this));
> ...
> }

Neat :)

R

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

On 30.05.2012 17:14, Regan Heath wrote:
>> synchronized class Container
>> {
>>
>> void applyLocked(scope delegate void(Container _this));
>> ...
>> }
>
> Neat :)
>
Rewritten and almost complete:

synchronized class Locked!(C)
{
	void applyLocked(scope delegate void(C _this) dg)
	{//should have lock/unlock already, as every sync'ed class
		gd(_this); 	
	}

	final auto opDispatch(string mtd, Args...)(Args args)
	{//can't go alias this, it won't be locked I think
		return mixin("_this."~mtd~"("~args~")");
	}
private:
	C _this;
}


-- 
Dmitry Olshansky

Le 30/05/2012 14:43, Regan Heath a écrit :
> On Wed, 30 May 2012 00:10:19 +0100, Jonathan M Davis
> <jmdavisProg@gmx.com> wrote:
>
>> On Wednesday, May 30, 2012 01:02:53 Alex Rønne Petersen wrote:
>>> But mutexes allow proper encapsulation by hiding the mutex resource. As
>>> I've proven in the very OP of this thread, both druntime and phobos
>>> suffer from the anti-pattern that is locking on a public resource when
>>> you shouldn't. The language encourages doing it with this synchronized
>>> business.
>>
>>> From your comments, it sounds to me like the problem is entirely with
>> synchronized blocks, not sychronized classes. Synchronized classes
>> don't have
>> the issue of having externals lock on them without the explicit use of
>> synchronized blocks. They're the equivalent of locking a mutex in
>> every public
>> function and releasing it afterwards. The problem is that that mutex is
>> effectively public such that when a synchronized block is used on it,
>> it locks
>> on the same mutex. If no synchronized blocks are used, as far as I can
>> tell,
>> it's a non-issue.
>>
>> As such, wouldn't simply making the use of a sychronized block on a
>> synchronized object illegal fix the problem?
>
> 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(...)
>
> obtains and releases the mutex twice each, and in between another thread
> could call o.insert() and insert the missing object - resulting in 2
> copies being inserted (or a duplicate insert exception being throw, or...).
>
> Now, the above is bad design, but the only other choice is to expose
> lock() and unlock() methods (or the mutex - back to square 1one) to
> allow locking around multiple class, but now the caller has to be
> careful to call unlock in all code paths. scope helps us in D, but this
> is actually the whole point of synchronized blocks - 1. ensuring the
> unlock always happens and 2. making the scope of the locking visible in
> the code.
>
> R
>

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.

Le 30/05/2012 14:55, Dmitry Olshansky a écrit :
> On 30.05.2012 16:43, Regan Heath wrote:
>> On Wed, 30 May 2012 00:10:19 +0100, Jonathan M Davis
>> <jmdavisProg@gmx.com> wrote:
>>
>>> On Wednesday, May 30, 2012 01:02:53 Alex Rønne Petersen wrote:
>>>> But mutexes allow proper encapsulation by hiding the mutex resource. As
>>>> I've proven in the very OP of this thread, both druntime and phobos
>>>> suffer from the anti-pattern that is locking on a public resource when
>>>> you shouldn't. The language encourages doing it with this synchronized
>>>> business.
>>>
>>>> From your comments, it sounds to me like the problem is entirely with
>>> synchronized blocks, not sychronized classes. Synchronized classes
>>> don't have
>>> the issue of having externals lock on them without the explicit use of
>>> synchronized blocks. They're the equivalent of locking a mutex in
>>> every public
>>> function and releasing it afterwards. The problem is that that mutex is
>>> effectively public such that when a synchronized block is used on it,
>>> it locks
>>> on the same mutex. If no synchronized blocks are used, as far as I can
>>> tell,
>>> it's a non-issue.
>>>
>>> As such, wouldn't simply making the use of a sychronized block on a
>>> synchronized object illegal fix the problem?
>>
>> 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(...)
>>
>> obtains and releases the mutex twice each, and in between another thread
>> could call o.insert() and insert the missing object - resulting in 2
>> copies being inserted (or a duplicate insert exception being throw,
>> or...).
>>
>> Now, the above is bad design, but the only other choice is to expose
>> lock() and unlock() methods (or the mutex - back to square 1one) to
>> allow locking around multiple class, but now the caller has to be
>> careful to call unlock in all code paths. scope helps us in D, but this
>> is actually the whole point of synchronized blocks - 1. ensuring the
>> unlock always happens and 2. making the scope of the locking visible in
>> the code.
>>
> synchronized class Container
> {
>
> void applyLocked(scope delegate void(Container _this));
> ...
> }
>
>

I often ends up doing similar stuff when manipulating pair of actions. lock/unlock, connect/disconect, begin/commit/rollback, etc . . .

Le 30/05/2012 15:10, Regan Heath a écrit :
> 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
>

As the GC allocate by power of 2, this can go pretty bad. It did happen to me (once to be fair, it is probably not the every day problem) that my memory consumption exploded because of the monitor field.