[
https://issues.apache.org/jira/browse/ARTEMIS-1152?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jiri Danek updated ARTEMIS-1152:
--------------------------------
Attachment: ManagementServiceImpl.java.png
ActiveMQThreadPoolExecutor.java.png
> 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.next
> Reporter: Jiri Danek
> 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
(v6.3.15#6346)
