river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <j...@zeus.net.au>
Subject Re: code comment in TxnManagerImpl
Date Mon, 01 Apr 2013 11:16:04 GMT
This actually gets worse if the second thread is an internal non static 
class, since it gets an implicit 'this' reference to the partially 
constructed object.

Peter Firmstone wrote:
> Hmm, :|
>
> To quote 
> http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4:
>
>    A call to |start()| on a thread /happens-before/ any actions in the
>    started thread.
>
> <comment>
> But does that guarantee that construction of objects whose references 
> will be written to final fields (guaranteed after construction 
> completes) in the constructor of an object that starts that thread, 
> will happen before or after the new thread is started?  Remembering 
> the jvm is free to not initialize and reorder something it doesn't 
> think it needs now, but must after construction is complete.
>
> So in other words the second thread which started during object 
> construction might not see the objects the first thread has created in 
> a fully constructed state as they haven't yet been published.
> </comment>
>
>    In some cases, such as deserialization, the system will need to
>    change the |final| fields of an object after construction. |final|
>    fields can be changed via reflection and other
>    implementation-dependent means. The only pattern in which this has
>    reasonable semantics is one in which an object is constructed and
>    then the |final| fields of the object are updated. The object should
>    not be made visible to other threads, nor should the |final| fields
>    be read, until all updates to the |final| fields of the object are
>    complete. Freezes of a |final| field occur both at the end of the
>    constructor in which the |final| field is set, and immediately after
>    each modification of a |final| field via reflection or other special
>    mechanism.
>
>    Even then, there are a number of complications. If a |final| field
>    is initialized to a compile-time constant expression (ยง15.28
>    
> <http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.28>)
>    in the field declaration, changes to the |final| field may not be
>    observed, since uses of that |final| field are replaced at compile
>    time with the value of the constant expression.
>
>    Another problem is that the specification allows aggressive
>    optimization of |final| fields. Within a thread, it is permissible
>    to reorder reads of a |final| field with those modifications of a
>    |final| field that do not take place in the constructor.
>
>
>    An implementation may provide a way to execute a block of code in a
>    /|final|-field-safe context/. If an object is constructed within a
>    |final|-field-safe context, the reads of a |final| field of that
>    object will not be reordered with modifications of that |final|
>    field that occur within that |final|-field-safe context.
>
>    A |final|-field-safe context has additional protections. If a thread
>    has seen an incorrectly published reference to an object that allows
>    the thread to see the default value of a |final||final|-field-safe
>    context, reads a properly published reference to the object, it will
>    be guaranteed to see the correct value of the |final| field. In the
>    formalism, code executed within a |final|-field-safe context is
>    treated as a separate thread (for the purposes of |final| field
>    semantics only).
>
>    In an implementation, a compiler should not move an access to a
>    |final| field into or out of a |final|-field-safe context (although
>    it can be moved around the execution of such a context, so long as
>    the object is not constructed within that context).
>
>
>
>
> Dan Creswell wrote:
>> On 1 April 2013 09:24, Peter Firmstone <jini@zeus.net.au> wrote:
>>
>>  
>>> Dan Creswell wrote:
>>>
>>>    
>>>> On 1 April 2013 08:11, Peter Firmstone <jini@zeus.net.au> wrote:
>>>>
>>>>
>>>>
>>>>      
>>>>> Food for thought:  After our pending release, it might be an idea 
>>>>> to make
>>>>> a combined effort to identify and address as many concurrency 
>>>>> issues as
>>>>> possible, we need to modernize our implementation code so we stay
>>>>> relevant.
>>>>>
>>>>> An important task will be updating all our service implementations so
>>>>> they
>>>>> DON'T start threads during construction.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>         
>>>> The ActiveObject pattern often does start threads at construction. I'd
>>>> like
>>>> to understand why that is such a problem for you? It surely isn't a 
>>>> big
>>>> deal for me but....
>>>>
>>>>
>>>>
>>>>       
>>> It allows fields to be declared final, if a thread is started during
>>> construction the JMM makes no guarantee that thread will see the final
>>> state of that objects fields after construction completes.
>>>
>>>     
>>
>> Not sure that's true, at least in JDK 7:
>>
>> http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4
>>
>> "An action that starts a thread *synchronizes-with* the first action 
>> in the
>> thread it starts. "
>>
>> "Two actions can be ordered by a *happens-before* relationship. If one
>> action *happens-before* another, then the first is visible to and 
>> ordered
>> before the second. "
>>
>> "If an action *x* *synchronizes-with* a following action *y*, then we 
>> also
>> have *hb(x, y)*. "
>>
>> i.e. If thread A is doing construction and then starts another thread,
>> variable assignments prior will be visible to the newly created thread.
>>
>> That in turn means so long as all critical assignments are done prior to
>> starting that second thread, there's no problem?
>>
>> And if that's true, starting a thread in a constructor needn't be 
>> avoided,
>> merely done "carefully". Thus it would be sufficient to ensure all final
>> variables are assigned prior to thread starting, which isn't so hard 
>> to do
>> or assure. I guess my point is, yes there's some care required but 
>> outright
>> banning thread start() in constructors is overkill.
>>
>> ?
>>
>>
>>  
>>> This is important when that thread accesses fields in the constructed
>>> object.
>>>
>>> See:
>>> https://www.securecoding.cert.**org/confluence/display/java/**
>>> TSM03-J.+Do+not+publish+**partially+initialized+objects<https://www.securecoding.cert.org/confluence/display/java/TSM03-J.+Do+not+publish+partially+initialized+objects>

>>>
>>> https://www.securecoding.cert.**org/confluence/display/java/**
>>> TSM01-J.+Do+not+let+the+this+**reference+escape+during+**
>>> object+construction<https://www.securecoding.cert.org/confluence/display/java/TSM01-J.+Do+not+let+the+this+reference+escape+during+object+construction>

>>>
>>>
>>> This doesn't mean you can't start a thread during construction, but it
>>> does mean you must be very careful if you do; our old code isn't that
>>> careful. ;)
>>>
>>> Cheers,
>>>
>>> Peter.
>>>
>>>     
>>
>>   
>
>


Mime
View raw message