Hi
It is true, but the case we have does not manipulate _navigationCases
directly, instead an new map is created and filled each time and only
before end the synchronized block it is assigned. In this case we
don't have any additional manipulation of _navigationCases after that
too.
regards,
Leonardo
2011/10/26 Mark Struberg <strub...@yahoo.de>:
>
>
> 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 <lu4...@gmail.com>
>> To: MyFaces Development <dev@myfaces.apache.org>; Mark Struberg
>> <strub...@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 <strub...@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 <lu4...@gmail.com>
>>>> To: myfaces-dev <dev@myfaces.apache.org>; Mark Struberg
>> <strub...@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 <strub...@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 <lu4...@gmail.com>
>>>>>> To: MyFaces Development <dev@myfaces.apache.org>; Mark
>> Struberg
>>>> <strub...@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 <strub...@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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
