View mode: basic / threaded / horizontal-split · Log in · Help
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 30-05-2012 20:25, Andrei Alexandrescu wrote:
> On 5/30/12 10:28 AM, Alex Rønne Petersen wrote:
>> On 30-05-2012 19:12, Andrei Alexandrescu wrote:
>>> On 5/30/12 9:23 AM, Alex Rønne Petersen wrote:
>>>> On 30-05-2012 18:14, Andrei Alexandrescu wrote:
>>>>> This is news to me. How do you publicly access the mutex of a
>>>>> synchronized class object?
>>>>
>>>> Generally in two ways:
>>>>
>>>> 1) synchronized (obj)
>>>
>>> This is not accessing the mutex for arbitrary operations.
>>
>> No, it is indeed not. You didn't explicitly say you wanted to do
>> "arbitrary operations", so I assumed that by accessing the mutex you
>> meant doing the only two things that are sensible for a mutex: Locking
>> and unlocking it. What else would you want to do?
>
> For example, locking it in one function and unlocking it in another.
> This is impossible with synchronized(obj).
>
> To summarize, the synchronized(obj) operation is not exposing the mutex,
> only manipulate it in a specific way.

It clearly exposes it *enough* to cause potential for deadlocks by 
manipulating it in this specific way, which is what this is all about.

I don't think arguing about what "exposed" means in this specific 
context (or perhaps in general?) or what levels of manipulation justify 
the use of the term "expose" will get the discussion anywhere...

>
>
> Andrei

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On Wednesday, 30 May 2012 at 15:45:05 UTC, Andrei Alexandrescu 
wrote:
> On 5/30/12 2:14 AM, Jacob Carlborg wrote:
>> It seems more and more that D2 is not a designed language. 
>> Instead new
>> features are just slapped on without considering how it would 
>> impact the
>> rest of the language.
>
> What features are you referring to?

The concurrency model as this thread shows. There is no bridge 
between shared and unshared data like const is to immutable and 
mutable. Perhaps the monitor on every object should have been 
removed when the new concurrency model was designed, as this 
thread suggests.

"inout" was added long after the const system was added to D. If 
done correctly this should have come up as a problem when 
designing the const system. You cannot apply const/immutable to 
an object reference in the same way as you can to a pointer.

It took some pretty good convincing for you to accept that 
strings weren't enough as a substitute for lambdas. Fortunately 
we have proper lambdas now :)

"const" doesn't play nice with ranges.

All these points have been mentioned before.

--
/Jacob Carlborg
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 30-05-2012 20:26, Martin Nowak wrote:
> On Tue, 29 May 2012 00:36:13 +0200, Alex Rønne Petersen
> <alex@lycus..org> wrote:
>
>> Hi,
>>
>> I've seen several occurrences of synchronized (this) and synchronized
>> (this.classinfo) in both druntime and phobos by now. I propose that we
>> officially ban these patterns with extreme prejudice.
>>
>> 1) Locking on the object instance is a HORRIBLE IDEA. Anyone who
>> happens to use the object for locking will most likely end up with a
>> deadlock on their hands.
>
>> 2) Locking on something as fundamental as type info means that any
>> arbitrary part of the application could cause a deadlock by doing the
>> same.
> All occurences of this pattern use it to obtain a global unique nameable
> mutex.
> It's a little ugly but I fail to see fundamental issues.
>
> synchronized(Stacktrace.classinfo)
> synchronized(typeid(ArrayAllocLengthLock)) // uses an exclusive type as
> named mutex
> synchronized(TaskPool.classinfo)
> synchronized(this.classinfo) // this a.k.a. Socket
>
> Then there is std.concurrency.registryLock which is used in the exact
> same ways as the occurences above. To do that it requires 6 lines of
> code, has to successfully avoid 2 pitfalls and allocates the mutex eagerly.
>
> Then there is core.thread which has a lot of issues with locking.
>
> There is some suspicious code in object.rt_attachDisposeEvent
> which synchronizes on it's argument.
>
> I'm inclined to say that this is a very thin backing for such a rant.

Also, I don't think calling this a rant is fair. I gave specific reasons 
for why the patterns I mentioned are bad in _libraries_ of all things 
(notice that I was talking about druntime and phobos!).

>
>
> There was one interesting argument by Regan Heath:
>
>> 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.
>
> Now I do not see anyone synchronizing it's code on arbitrary objects or any
> other kind of abuse that would increase the inherent possibility for
> deadlocks.


-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On Wed, 30 May 2012 14:32:59 -0400, Andrei Alexandrescu  
<SeeWebsiteForEmail@erdani.org> wrote:

> On 5/30/12 10:47 AM, Steven Schveighoffer wrote:
>> Yes, you can just use a private mutex. But doesn't that just lead to
>> recommending not using a feature of the language?
>
> I don't think so. Synchronized classes are the unit of scoped locking in  
> D. If you want to do all scoped locking internally, make the class  
> private.

Maybe you didn't read thoroughly the first part of my post (the example  
that shows a deadlock that is easily preventable if the mutex isn't  
exposed).  The issue is that it's possible to hijack the mutex that you  
are using to protect the class data in order to protect *external* data.   
Hijacking is not possible if the mutex is private (i.e. a private member).

So great!  Just don't use the builtin object mutex, because it exposes  
possible race conditions.  But then why do we have it (the object hidden  
mutex)?  And now I can't use synchronized classes to ensure the whole  
thing is protected, I have to write all my functions like this:

void foo()
{
   synchronized(this.mutex) // lock a private mutex
   {
   }
}

-Steve
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 5/30/12 11:38 AM, Alex Rønne Petersen wrote:
> On 30-05-2012 20:25, Andrei Alexandrescu wrote:
>> To summarize, the synchronized(obj) operation is not exposing the mutex,
>> only manipulate it in a specific way.
>
> It clearly exposes it *enough* to cause potential for deadlocks by
> manipulating it in this specific way, which is what this is all about.

Nobody claimed synchronized protects against deadlocks. Unfortunately, 
raw mutexes are just as bad in that regard.

> I don't think arguing about what "exposed" means in this specific
> context (or perhaps in general?) or what levels of manipulation justify
> the use of the term "expose" will get the discussion anywhere...

I simply pointed out a factual mistake in your argument.


Andrei
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 5/30/12 11:41 AM, Jacob Carlborg wrote:
> On Wednesday, 30 May 2012 at 15:45:05 UTC, Andrei Alexandrescu wrote:
>> On 5/30/12 2:14 AM, Jacob Carlborg wrote:
>>> It seems more and more that D2 is not a designed language. Instead new
>>> features are just slapped on without considering how it would impact the
>>> rest of the language.
>>
>> What features are you referring to?
>
> The concurrency model as this thread shows. There is no bridge between
> shared and unshared data like const is to immutable and mutable.

We considered that (maybe_synchronized) , but decided not to go with it 
amid fear of overcomplicating things.

> Perhaps
> the monitor on every object should have been removed when the new
> concurrency model was designed, as this thread suggests.

This thread does a good job at arguing that scoped locking does not 
prevent deadlocks, but this is not new or interesting. Unfortunately I 
fail to derive significant proposed value.

There exist type systems that avoid locks. They are very restrictive and 
difficult to work with.

> "inout" was added long after the const system was added to D. If done
> correctly this should have come up as a problem when designing the const
> system. You cannot apply const/immutable to an object reference in the
> same way as you can to a pointer.
>
> "const" doesn't play nice with ranges.

I see how these can be annoying, but they're not the result of us not 
designing things. We designed things best we could.


Andrei
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 5/30/12 12:03 PM, Steven Schveighoffer wrote:
> On Wed, 30 May 2012 14:32:59 -0400, Andrei Alexandrescu
> <SeeWebsiteForEmail@erdani.org> wrote:
>
>> On 5/30/12 10:47 AM, Steven Schveighoffer wrote:
>>> Yes, you can just use a private mutex. But doesn't that just lead to
>>> recommending not using a feature of the language?
>>
>> I don't think so. Synchronized classes are the unit of scoped locking
>> in D. If you want to do all scoped locking internally, make the class
>> private.
>
> Maybe you didn't read thoroughly the first part of my post (the example
> that shows a deadlock that is easily preventable if the mutex isn't
> exposed).

The mutex is not exposed.

> The issue is that it's possible to hijack the mutex that you
> are using to protect the class data in order to protect *external* data.
> Hijacking is not possible if the mutex is private (i.e. a private member).
>
> So great! Just don't use the builtin object mutex, because it exposes
> possible race conditions. But then why do we have it (the object hidden
> mutex)? And now I can't use synchronized classes to ensure the whole
> thing is protected, I have to write all my functions like this:
>
> void foo()
> {
> synchronized(this.mutex) // lock a private mutex
> {
> }
> }

This is a good example. But many synchronized classes are actually 
meaningful (e.g. the classic synchronized queue etc).


Andrei
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
Le 30/05/2012 17:50, Andrei Alexandrescu a écrit :
> On 5/30/12 2:40 AM, deadalnix wrote:
>> Le 30/05/2012 00:50, Andrei Alexandrescu a écrit :
>>> On 5/29/12 2:59 PM, Alex Rønne Petersen wrote:
>>>> On 29-05-2012 23:31, Andrei Alexandrescu wrote:
>>>>> On 5/29/12 1:32 AM, deadalnix wrote:
>>>>>> I already did some comment about this.
>>>>>>
>>>>>> Making any object synchronization is a very bad design decision.
>>>>>> It is
>>>>>> deadlock prone, it is liquid lock prone, it cause an overhead on any
>>>>>> object, and even worse, it is useless most of the time as D promote
>>>>>> thread locality (which is very good).
>>>>>
>>>>> Actually I think such a characterization is superficial and biased to
>>>>> the extent it becomes wrong.
>>>>
>>>> Really ? I think he's spot on.
>>>
>>> I'd be glad to think the same. Good arguments might be helpful. So far
>>> I've seen exaggerated statements and posturing. Such are entertaining
>>> but are of limited effectiveness in furthering a point.
>>>
>>
>> I have provided link in other posts to document the point. Not to
>> mention that my personal experience back up this too.
>>
>> I don't like what you are doing here, because you don't provide anything
>> and simply discard what don't fit the current design. For instance, you
>> didn't even considered the liquid lock problem.
>
> It would help if I knew what a liquid lock is. The first page of Google
> search doesn't help.
>
> I'm only discarding content-less posturing a la "worst idea ever",
> "useless", "very bad design decision" etc. Such is just immature. It is
> encouraging you have started sending content (the msdn paper), thanks,
> and please keep it coming.
>
>
> Thanks,
>
> Andrei

Ok let's me explain what liquid lock is. It is a common error in 
languages that allow lock on any object.

The phenomena is fairly simple, but lead to really tedious debugging 
session. It happen when you use an object that is useful for some 
functionality of your program as a mutex. This is OK because the 
instance is always the same. Then, later on, when adding new 
functionality/refactoring/whatever, you introduce a possibility to 
change the instance you lock on.

In such situation, you seems to lock in an appropriate manner, but you 
fail to ensure correct locking when it occur at the exact moment the 
object referred is changed.

This is really tedious to track down, and quite easy to create when 
several developers are working on the same codebase for an extended 
period of time. To avoid this, you must ensure that any reference on 
object you mutate isn't used somewhere as a mutex.

It is considered good practice in Java to not lock on any object and use 
a specific object to lock on. The reference to that object will be 
final, so it is guaranteed to not change.

For instance, Intellij issue a warning about that : 
http://www.coderanch.com/t/233943/threads/java/Synchronization-non-final-field

At the end, locking on any object cause trouble.

Locking on any object has proven itself to be a very bad design decision 
in Java and C#. I don't think it is immature to say that. This is well 
known in java community, and I guess it is too in C# (I have way less 
experience with C# ).
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
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 missed that message. I don't think synchronized classes should go 
away. I think synchronize on non synchronized classes should go away.
May 30, 2012
Re: synchronized (this[.classinfo]) in druntime and phobos
On 30-05-2012 21:10, Andrei Alexandrescu wrote:
> On 5/30/12 11:41 AM, Jacob Carlborg wrote:
>> On Wednesday, 30 May 2012 at 15:45:05 UTC, Andrei Alexandrescu wrote:
>>> On 5/30/12 2:14 AM, Jacob Carlborg wrote:
>>>> It seems more and more that D2 is not a designed language. Instead new
>>>> features are just slapped on without considering how it would impact
>>>> the
>>>> rest of the language.
>>>
>>> What features are you referring to?
>>
>> The concurrency model as this thread shows. There is no bridge between
>> shared and unshared data like const is to immutable and mutable.
>
> We considered that (maybe_synchronized) , but decided not to go with it
> amid fear of overcomplicating things.

The result is a feature that is arguably only useful in small laboratory 
cases. Are you sure we shouldn't revisit shared and try to work out a 
bridge between the unshared and shared world?

Not to mention that shared is fundamentally x86-biased, which seems to 
be annoying trend in D development in general...

>
>> Perhaps
>> the monitor on every object should have been removed when the new
>> concurrency model was designed, as this thread suggests.
>
> This thread does a good job at arguing that scoped locking does not
> prevent deadlocks, but this is not new or interesting. Unfortunately I
> fail to derive significant proposed value.
>
> There exist type systems that avoid locks. They are very restrictive and
> difficult to work with.

I can think of 3 languages that have monitors in the "type" system: D, 
C# (as a result of the CLR's design; mostly because they wanted Java 
interoperability), and Java. Are you really saying all other type 
systems are difficult to work with just because of this little thing?

>
>> "inout" was added long after the const system was added to D. If done
>> correctly this should have come up as a problem when designing the const
>> system. You cannot apply const/immutable to an object reference in the
>> same way as you can to a pointer.
>>
>> "const" doesn't play nice with ranges.
>
> I see how these can be annoying, but they're not the result of us not
> designing things. We designed things best we could.
>
>
> Andrei

-- 
Alex Rønne Petersen
alex@lycus.org
http://lycus.org
11 12 13 14 15 16 17 18 19
Top | Discussion index | About this forum | D home