myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Struberg <strub...@yahoo.de>
Subject Re: MYFACES-3368 status
Date Wed, 26 Oct 2011 22:04:01 GMT


This is an argument which I hear often. But it's actually wrong. Let's assume:


Map getX() {
  if (x=null) {
    synchronized(this) {

      if (x=null) {
        x=new HashMap();
      }

    }
  }
 return x;
}


People argue that the worst what can happen is that x will get created twice. But actually
that is wrong.
The worst case is that thread A creates a new x HashMap and returns it. And then the caller
method will add some values.
And this values will be missing if concurrent thread B creates a new HashMap and 'replaces'
x.
This will only work if x is declared volatile, otherwise the first x==null check might not
see the 'real' value.


LieGrue,
strub



----- Original Message -----
> From: Leonardo Uribe <lu4242@gmail.com>
> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg <struberg@yahoo.de>
> Cc: 
> Sent: Wednesday, October 26, 2011 11:43 PM
> Subject: Re: MYFACES-3368 status
> 
> Hi
> 
> In NavigationHandlerImpl#_navigationCases, that var should be
> volatile, but note if it is not nothing happens, just the code will be
> executed more times than necessary.
> 
> regards,
> 
> Leonardo Uribe
> 
> 2011/10/26 Mark Struberg <struberg@yahoo.de>:
>>  For example
>>  NavigationHandlerImpl#_navigationCases
>> 
>>  I already fixed a few occurrences.
>> 
>>  There are also a few other checkstyle errors waiting to get fixed like 
> methods which are 450 lines long..
>> 
>>  Would be cool if you could take a look at it - txs!
>> 
>> 
>>  LieGrue,
>>  strub
>> 
>> 
>> 
>>  ----- Original Message -----
>>>  From: Leonardo Uribe <lu4242@gmail.com>
>>>  To: myfaces-dev <dev@myfaces.apache.org>; Mark Struberg 
> <struberg@yahoo.de>
>>>  Cc:
>>>  Sent: Wednesday, October 26, 2011 10:45 PM
>>>  Subject: Re: MYFACES-3368 status
>>> 
>>>  Yes I know that, but where in myfaces we have a problem like that? so
>>>  we can discuss it in deep.
>>> 
>>>  2011/10/26 Mark Struberg <struberg@yahoo.de>:
>>>>   Again (think I posted this before already), a very good source for 
> java mem
>>>  spec behaviour is the comment of Brian Goetz
>>>> 
>>>>   http://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html
>>>> 
>>>>   This also explains why you need to declare the field volatile - 
> don't
>>>  wanna stress this, but I told this also before... ;)
>>>> 
>>>> 
>>>>   LieGrue,
>>>>   strub
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>>   ----- Original Message -----
>>>>>   From: Leonardo Uribe <lu4242@gmail.com>
>>>>>   To: MyFaces Development <dev@myfaces.apache.org>; Mark 
> Struberg
>>>  <struberg@yahoo.de>
>>>>>   Cc:
>>>>>   Sent: Wednesday, October 26, 2011 8:40 PM
>>>>>   Subject: Re: MYFACES-3368 status
>>>>> 
>>>>>   Hi
>>>>> 
>>>>>   I'm keeping an eye on the changes done. I don't 
> remember any
>>>  problem
>>>>>   about 'double lock' on myfaces. Could you please be 
> more
>>>  specific? In
>>>>>   my understanding everything works as expected.
>>>>> 
>>>>>   regards,
>>>>> 
>>>>>   Leonardo Uribe
>>>>> 
>>>>>   2011/10/26 Mark Struberg <struberg@yahoo.de>:
>>>>>>    Hi folks!
>>>>>> 
>>>>>>    I'm down to 24 checkstyle issues (with 
> LineLength=160,
>>>  otherwise ~600
>>>>>   more) which are really non-trivial.
>>>>>> 
>>>>>>    A few of them are 'equals() without hashCode()' 
> which are
>>>>>   definitely problematic
>>>>>>    Then we have a few variable namings which I didn't 
> want to
>>>  change
>>>>>   without talking to you.
>>>>>>    And of course there are some 'double lock idiom' 
> issues.
>>>>>>    It's safe to use double-locking as of Java5, but ONLY 
> if the
>>>  locked
>>>>>   variable is declared volatile! We barely have this, so I left 
> the check
>>>>>   enabled...
>>>>>> 
>>>>>> 
>>>>>>    Who is taking care of those remaining issues? Leo?
>>>>>> 
>>>>>> 
>>>>>>    LieGrue,
>>>>>>    strub
>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
>

Mime
View raw message