[ https://issues.apache.org/jira/browse/ARTEMIS-1152?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Timothy A. Bish closed ARTEMIS-1152. ------------------------------------ Resolution: Not A Problem Patterns appear to use correct volatile and locking state, reopen if further evidence is available > Investigate suspected Double-Checked Locking > -------------------------------------------- > > Key: ARTEMIS-1152 > URL: https://issues.apache.org/jira/browse/ARTEMIS-1152 > Project: ActiveMQ Artemis > Issue Type: Wish > Components: Broker > Affects Versions: 2.1.0 > Reporter: Jiri Daněk > Priority: Minor > Attachments: ActiveMQThreadPoolExecutor.java.png, > ManagementServiceImpl.java.png > > > Coverity found instance of Double-Checked Locking. According to the resources > at the end, the variable has to either be declared volatile, or > double-checked locking should not be used, or the variable has to be atomic > (int, float, ...). Otherwise, in a general case this may lead to threads > accessing partially initialized objects. > There is 9 Coverity finds in the category "Data race undermines locking", the > following one looks to me as clear examples of the Double-Checked Locking > pattern > h4. ActiveMQJMSContext.java > {noformat} > 130 private void checkSession() { > 1. thread1_checks_field: Thread1 uses the value read from field session > in the condition session == null. It sees that the condition is true. > > CID 1409080 (#2-1 of 2): Check of thread-shared field evades lock acquisition > (LOCK_EVASION) > 5. thread2_checks_field_early: Thread2 checks session, reading it after > Thread1 assigns to session but before some of the correlated field > assignments can occur. It sees the condition session == null as being false. > It continues on before the critical section has completed, and can read data > changed by that critical section while it is in an inconsistent state. > Remove this outer, unlocked check of session. > 131 if (session == null) { > 2. thread1_acquires_lock: Thread1 acquires lock ActiveMQJMSContext.this. > 132 synchronized (this) { > 133 if (closed) > 134 throw new IllegalStateRuntimeException("Context is closed"); > 3. thread1_double_checks_field: Thread1 double checks the field session > in the condition session == null. > 135 if (session == null) { > 136 try { > 137 if (xa) { > 138 session = ((XAConnection) > connection).createXASession(); > 139 } else { > 4. thread1_modifies_field: Thread1 modifies the field session. This > modification can be re-ordered with other correlated field assignments within > this critical section at runtime. Thus, checking the value of session is not > an adequate test that the critical section has completed unless the guarding > lock is held while checking. If session is assigned a newly constructed > value, note that the JVM is allowed to reorder the assignment of the new > reference to session before any field assignments that may occur in the > constructor. Control is switched to Thread2. > 140 session = connection.createSession(sessionMode); > 141 } > 142 } catch (JMSException e) { > 143 throw JmsExceptionUtils.convertToRuntimeException(e); > 144 } > 145 } > 146 } > 147 } > 148 } > {noformat} > In addition to that, the demo version of HP Fortify tool reports the > following two additional instances (as well as the previous one already > reported by Coverity). > h4. ActiveMQThreadPoolExecutor.java > !ActiveMQThreadPoolExecutor.java.png! > h4. ManagementServiceImpl.java > !ManagementServiceImpl.java.png! > I came to believe that this warning from the tool is real, at least the first > instance, and that it should be investigated more closely by somebody more > experienced. > #. http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html > #. > https://www.securecoding.cert.org/confluence/display/java/LCK10-J.+Use+a+correct+form+of+the+double-checked+locking+idiom -- This message was sent by Atlassian Jira (v8.20.10#820010)