lazy initialisation in MyFaces-core

[Mark Struberg <st...@yahoo.de>]

Hi!

checkstyle just found another kind of gems 


for example in AbstractAttributeMap.java


    private Collection<V> _values;

    @Override
    public Collection<V> values()
    {
        return (_values != null) ? _values : (_values = new Values());
    }

This code is not thread safe! 
_values is neither volatile nor is there any synchronized used on it!

What shall we do?
Either we declare _values volatile and use synchronized double lock idiom, or we just create the new Values() at class initialisation.

I don't know enough about the usage of this class to make any decission - who takes over?

LieGrue,
strub

Re: lazy initialisation in MyFaces-core

[Leonardo Uribe <lu...@gmail.com>]

Hi

There are two classes:

org.apache.myfaces.util.AbstractAttributeMap

and

org.apache.myfaces.util.AbstractThreadSafeAttributeMap

You're right, there is a bug, but apply them please on
AbstractThreadSafeAttributeMap. Note ApplicationMap and SessionMap
wrappers used by ExternalContext inherits from this class.

regards,

Leonardo Uribe

2011/10/26 Mark Struberg <st...@yahoo.de>:
> Hi!
>
> checkstyle just found another kind of gems
>
>
> for example in AbstractAttributeMap.java
>
>
>     private Collection<V> _values;
>
>     @Override
>     public Collection<V> values()
>     {
>         return (_values != null) ? _values : (_values = new Values());
>     }
>
> This code is not thread safe!
> _values is neither volatile nor is there any synchronized used on it!
>
> What shall we do?
> Either we declare _values volatile and use synchronized double lock idiom, or we just create the new Values() at class initialisation.
>
> I don't know enough about the usage of this class to make any decission - who takes over?
>
> LieGrue,
> strub
>
>