You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@river.apache.org by Peter Firmstone <ji...@zeus.net.au> on 2013/04/01 09:11:46 UTC
code comment in TxnManagerImpl
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.
Every time I fix one concurrency problem, performance usually improves
and another heisenbug appears.
/* Expiration checks are only meaningful for active transactions. */
/* NOTE:
* 1) Cancellation sets expiration to 0 without changing state
* from Active right away. Clients are supposed to treat
* UnknownTransactionException just like Aborted, so it's OK to send
* in this case.
* 2) Might be a small window where client is committing the transaction
* close to the expiration time. If the committed transition takes
* place between getState() and ensureCurrent then the client could get
* a false result.
*/
//TODO - need better locking here. getState and expiration need to be
checked atomically
Re: code comment in TxnManagerImpl
Posted by Gregg Wonderly <ge...@cox.net>.
On Apr 1, 2013, at 5:03 AM, Dan Creswell <da...@gmail.com> wrote:
> On 1 April 2013 09:24, Peter Firmstone <ji...@zeus.net.au> wrote:
>
>> Dan Creswell wrote:
>>
>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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
One caution to remember, is that the JLS only applies to intra-thread observable behaviors. When concurrency comes into play, you have to stare at the JMM too, to work out what reorderings might actually be possible.
Gregg Wonderly
> "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.
>>
Re: code comment in TxnManagerImpl
Posted by Gregg Wonderly <ge...@cox.net>.
As a casual observation, I'll add, that I've been learning more and more about intra-thread optimizations which reorder a lot more than what you might expect. Race conditions, in particular can now actually expose corrupt data and cause unexpected RuntimeExceptions. As an example, Thread.getName() and Thread.setName() have this problem because of optimizations around System.arraycopy optimizations.
The concurrency issues that are visible could be from side effects that are rooted in the reordering optimizations that we are not expecting, when just looking at the code as written. Reordering can be shutdown pretty readily by moving to the highly defensive position of saying "every field is either final or volatile". Doing that, as the first step in debugging concurrency might actually straighten out the behaviors to just be "race" conditions and make it a little easier to understand what is actually happening, if reordering is creating confusion.
Gregg Wonderly
On Apr 1, 2013, at 3:44 PM, Dan Creswell <da...@gmail.com> wrote:
> I'd call them guidelines not rules but fair enough ;)
>
> On 1 April 2013 21:10, Peter Firmstone <ji...@zeus.net.au> wrote:
>
>> Concurrency is a complex subject even experts struggle with, I know you're
>> a good programmer, I think the discussion will be more fruitful here, to be
>> honest, I'm just following coding rules that much smarter developers have
>> devised for me:
>>
>> Send Concurrency-interest mailing list submissions to
>> concurrency-interest@cs.**oswego.edu<co...@cs.oswego.edu>
>>
>> To subscribe or unsubscribe via the World Wide Web, visit
>> http://cs.oswego.edu/mailman/**listinfo/concurrency-interest<http://cs.oswego.edu/mailman/listinfo/concurrency-interest>
>> or, via email, send a message with subject or body 'help' to
>> concurrency-interest-request@**cs.oswego.edu<co...@cs.oswego.edu>
>>
>> You can reach the person managing the list at
>> concurrency-interest-owner@cs.**oswego.edu<co...@cs.oswego.edu>
>>
>>
>> Cheers,
>>
>> Peter.
>>
>> Dan Creswell wrote:
>>
>>> Dude,
>>>
>>>
>>> On 1 April 2013 12:10, Peter Firmstone <ji...@zeus.net.au> wrote:
>>>
>>>
>>>
>>>> Hmm, :|
>>>>
>>>> To quote http://docs.oracle.com/javase/****specs/jls/se7/html/jls-17.**<http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**>
>>>> html#jls-17.4<http://docs.**oracle.com/javase/specs/jls/**
>>>> se7/html/jls-17.html#jls-17.4<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.
>>>>
>>>>
>>>>
>>>>
>>> You gotta stop treating constructors like they're magic!
>>>
>>> A constructor has no special semantics in and of itself. It _appears_ like
>>> it does because there is one "special" thing that can be said of
>>> construction: It is dispatched within the context of single thread. Two
>>> separate calls from two separate threads to the constructor are isolated
>>> as
>>> a consequence. Constructors are not atomic, confer nothing in terms of
>>> ordering, do not represent a synchronization action or have any other
>>> impact on threading.
>>>
>>> So, constructors aren't special, they merely have a set of behaviours
>>> implied by the language specification. One of those amounts to:
>>>
>>> If a programmer does nothing thread-impacting in a constructor, the only
>>> way any other thread gets to see the object is through the creating thread
>>> making a reference available to another thread. In which case, there has
>>> been a synchronization which thus causes the entirety of actions in the
>>> constructor to be visible all at once. This "all at once" is because the
>>> constructor must have completed before the reference was made visible to
>>> another thread.
>>>
>>> But *if* a programmer does do some thread stuff in the constructor, then
>>> normal rules of synchronization apply. Amongst other things that means:
>>>
>>> (1) You can't re-order across a synchronization point.
>>> (2) All changes prior to a synchronization point will be made visible at
>>> that synchronization point.
>>>
>>> start() is a synchronization point and thus anything done before it is
>>> made
>>> visible to all threads once it's completed. Further start() completes
>>> before the new thread starts (noting that there may be an immediate
>>> context
>>> switch at that point such that the parent thread takes no further action
>>> but the synchronization has happened prior to the switch).
>>>
>>> Thus, all finals initialized prior to calling start() will be visible to
>>> the new thread. They *may* have been re-ordered for processor performance
>>> reasons (e.g. to keep pipelines full).
>>>
>>>
>>>
>>>
>>>
>>>> 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-****<http://docs.oracle.com/**javase/specs/jls/se7/html/jls-**>
>>>> 15.html#jls-15.28<http://docs.**oracle.com/javase/specs/jls/**
>>>> se7/html/jls-15.html#jls-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 <ji...@zeus.net.au> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> Dan Creswell wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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<http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**html#jls-17.4>
>>>>> <http://docs.**oracle.com/javase/specs/jls/**
>>>>> se7/html/jls-17.html#jls-17.4<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/**
>>>>>> 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/**<http://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<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.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
Re: code comment in TxnManagerImpl
Posted by Dan Creswell <da...@gmail.com>.
I'd call them guidelines not rules but fair enough ;)
On 1 April 2013 21:10, Peter Firmstone <ji...@zeus.net.au> wrote:
> Concurrency is a complex subject even experts struggle with, I know you're
> a good programmer, I think the discussion will be more fruitful here, to be
> honest, I'm just following coding rules that much smarter developers have
> devised for me:
>
> Send Concurrency-interest mailing list submissions to
> concurrency-interest@cs.**oswego.edu<co...@cs.oswego.edu>
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://cs.oswego.edu/mailman/**listinfo/concurrency-interest<http://cs.oswego.edu/mailman/listinfo/concurrency-interest>
> or, via email, send a message with subject or body 'help' to
> concurrency-interest-request@**cs.oswego.edu<co...@cs.oswego.edu>
>
> You can reach the person managing the list at
> concurrency-interest-owner@cs.**oswego.edu<co...@cs.oswego.edu>
>
>
> Cheers,
>
> Peter.
>
> Dan Creswell wrote:
>
>> Dude,
>>
>>
>> On 1 April 2013 12:10, Peter Firmstone <ji...@zeus.net.au> wrote:
>>
>>
>>
>>> Hmm, :|
>>>
>>> To quote http://docs.oracle.com/javase/****specs/jls/se7/html/jls-17.**<http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**>
>>> html#jls-17.4<http://docs.**oracle.com/javase/specs/jls/**
>>> se7/html/jls-17.html#jls-17.4<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.
>>>
>>>
>>>
>>>
>> You gotta stop treating constructors like they're magic!
>>
>> A constructor has no special semantics in and of itself. It _appears_ like
>> it does because there is one "special" thing that can be said of
>> construction: It is dispatched within the context of single thread. Two
>> separate calls from two separate threads to the constructor are isolated
>> as
>> a consequence. Constructors are not atomic, confer nothing in terms of
>> ordering, do not represent a synchronization action or have any other
>> impact on threading.
>>
>> So, constructors aren't special, they merely have a set of behaviours
>> implied by the language specification. One of those amounts to:
>>
>> If a programmer does nothing thread-impacting in a constructor, the only
>> way any other thread gets to see the object is through the creating thread
>> making a reference available to another thread. In which case, there has
>> been a synchronization which thus causes the entirety of actions in the
>> constructor to be visible all at once. This "all at once" is because the
>> constructor must have completed before the reference was made visible to
>> another thread.
>>
>> But *if* a programmer does do some thread stuff in the constructor, then
>> normal rules of synchronization apply. Amongst other things that means:
>>
>> (1) You can't re-order across a synchronization point.
>> (2) All changes prior to a synchronization point will be made visible at
>> that synchronization point.
>>
>> start() is a synchronization point and thus anything done before it is
>> made
>> visible to all threads once it's completed. Further start() completes
>> before the new thread starts (noting that there may be an immediate
>> context
>> switch at that point such that the parent thread takes no further action
>> but the synchronization has happened prior to the switch).
>>
>> Thus, all finals initialized prior to calling start() will be visible to
>> the new thread. They *may* have been re-ordered for processor performance
>> reasons (e.g. to keep pipelines full).
>>
>>
>>
>>
>>
>>> 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-****<http://docs.oracle.com/**javase/specs/jls/se7/html/jls-**>
>>> 15.html#jls-15.28<http://docs.**oracle.com/javase/specs/jls/**
>>> se7/html/jls-15.html#jls-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 <ji...@zeus.net.au> wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> Dan Creswell wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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<http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**html#jls-17.4>
>>>> <http://docs.**oracle.com/javase/specs/jls/**
>>>> se7/html/jls-17.html#jls-17.4<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/**
>>>>> 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/**<http://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<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.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>
Re: code comment in TxnManagerImpl
Posted by Peter Firmstone <ji...@zeus.net.au>.
Concurrency is a complex subject even experts struggle with, I know
you're a good programmer, I think the discussion will be more fruitful
here, to be honest, I'm just following coding rules that much smarter
developers have devised for me:
Send Concurrency-interest mailing list submissions to
concurrency-interest@cs.oswego.edu
To subscribe or unsubscribe via the World Wide Web, visit
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
or, via email, send a message with subject or body 'help' to
concurrency-interest-request@cs.oswego.edu
You can reach the person managing the list at
concurrency-interest-owner@cs.oswego.edu
Cheers,
Peter.
Dan Creswell wrote:
> Dude,
>
>
> On 1 April 2013 12:10, Peter Firmstone <ji...@zeus.net.au> wrote:
>
>
>> Hmm, :|
>>
>> To quote http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**
>> html#jls-17.4<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.
>>
>>
>>
> You gotta stop treating constructors like they're magic!
>
> A constructor has no special semantics in and of itself. It _appears_ like
> it does because there is one "special" thing that can be said of
> construction: It is dispatched within the context of single thread. Two
> separate calls from two separate threads to the constructor are isolated as
> a consequence. Constructors are not atomic, confer nothing in terms of
> ordering, do not represent a synchronization action or have any other
> impact on threading.
>
> So, constructors aren't special, they merely have a set of behaviours
> implied by the language specification. One of those amounts to:
>
> If a programmer does nothing thread-impacting in a constructor, the only
> way any other thread gets to see the object is through the creating thread
> making a reference available to another thread. In which case, there has
> been a synchronization which thus causes the entirety of actions in the
> constructor to be visible all at once. This "all at once" is because the
> constructor must have completed before the reference was made visible to
> another thread.
>
> But *if* a programmer does do some thread stuff in the constructor, then
> normal rules of synchronization apply. Amongst other things that means:
>
> (1) You can't re-order across a synchronization point.
> (2) All changes prior to a synchronization point will be made visible at
> that synchronization point.
>
> start() is a synchronization point and thus anything done before it is made
> visible to all threads once it's completed. Further start() completes
> before the new thread starts (noting that there may be an immediate context
> switch at that point such that the parent thread takes no further action
> but the synchronization has happened prior to the switch).
>
> Thus, all finals initialized prior to calling start() will be visible to
> the new thread. They *may* have been re-ordered for processor performance
> reasons (e.g. to keep pipelines full).
>
>
>
>
>> 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<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 <ji...@zeus.net.au> wrote:
>>>
>>>
>>>
>>>
>>>> Dan Creswell wrote:
>>>>
>>>>
>>>>
>>>>
>>>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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<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/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<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.
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>
>
Re: code comment in TxnManagerImpl
Posted by Dan Creswell <da...@gmail.com>.
Dude,
On 1 April 2013 12:10, Peter Firmstone <ji...@zeus.net.au> wrote:
> Hmm, :|
>
> To quote http://docs.oracle.com/javase/**specs/jls/se7/html/jls-17.**
> html#jls-17.4<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.
>
>
You gotta stop treating constructors like they're magic!
A constructor has no special semantics in and of itself. It _appears_ like
it does because there is one "special" thing that can be said of
construction: It is dispatched within the context of single thread. Two
separate calls from two separate threads to the constructor are isolated as
a consequence. Constructors are not atomic, confer nothing in terms of
ordering, do not represent a synchronization action or have any other
impact on threading.
So, constructors aren't special, they merely have a set of behaviours
implied by the language specification. One of those amounts to:
If a programmer does nothing thread-impacting in a constructor, the only
way any other thread gets to see the object is through the creating thread
making a reference available to another thread. In which case, there has
been a synchronization which thus causes the entirety of actions in the
constructor to be visible all at once. This "all at once" is because the
constructor must have completed before the reference was made visible to
another thread.
But *if* a programmer does do some thread stuff in the constructor, then
normal rules of synchronization apply. Amongst other things that means:
(1) You can't re-order across a synchronization point.
(2) All changes prior to a synchronization point will be made visible at
that synchronization point.
start() is a synchronization point and thus anything done before it is made
visible to all threads once it's completed. Further start() completes
before the new thread starts (noting that there may be an immediate context
switch at that point such that the parent thread takes no further action
but the synchronization has happened prior to the switch).
Thus, all finals initialized prior to calling start() will be visible to
the new thread. They *may* have been re-ordered for processor performance
reasons (e.g. to keep pipelines full).
> 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<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 <ji...@zeus.net.au> wrote:
>>
>>
>>
>>> Dan Creswell wrote:
>>>
>>>
>>>
>>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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<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/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<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.
>>>
>>>
>>>
>>
>>
>>
>
>
Re: code comment in TxnManagerImpl
Posted by Peter Firmstone <ji...@zeus.net.au>.
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 <ji...@zeus.net.au> wrote:
>>
>>
>>> Dan Creswell wrote:
>>>
>>>
>>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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.
>>>
>>>
>>
>>
>
>
Re: code comment in TxnManagerImpl
Posted by Peter Firmstone <ji...@zeus.net.au>.
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 <ji...@zeus.net.au> wrote:
>
>
>> Dan Creswell wrote:
>>
>>
>>> On 1 April 2013 08:11, Peter Firmstone <ji...@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.
>>
>>
>
>
Re: code comment in TxnManagerImpl
Posted by Dan Creswell <da...@gmail.com>.
On 1 April 2013 09:24, Peter Firmstone <ji...@zeus.net.au> wrote:
> Dan Creswell wrote:
>
>> On 1 April 2013 08:11, Peter Firmstone <ji...@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.
>
Re: code comment in TxnManagerImpl
Posted by Peter Firmstone <ji...@zeus.net.au>.
Dan Creswell wrote:
> On 1 April 2013 08:11, Peter Firmstone <ji...@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.
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/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.
Re: code comment in TxnManagerImpl
Posted by Dan Creswell <da...@gmail.com>.
On 1 April 2013 08:11, Peter Firmstone <ji...@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....
> Every time I fix one concurrency problem, performance usually improves and
> another heisenbug appears.
>
> /* Expiration checks are only meaningful for active transactions. */
> /* NOTE:
> * 1) Cancellation sets expiration to 0 without changing state
> * from Active right away. Clients are supposed to treat
> * UnknownTransactionException just like Aborted, so it's OK to send
> * in this case.
> * 2) Might be a small window where client is committing the transaction
> * close to the expiration time. If the committed transition takes
> * place between getState() and ensureCurrent then the client could get
> * a false result.
> */
> //TODO - need better locking here. getState and expiration need to be
> checked atomically
>
This sort of thing is a big deal for me but has nothing much to do with
starting threads in constructors....