activemq-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jiri Danek (JIRA)" <j...@apache.org>
Subject [jira] [Created] (ARTEMIS-1152) Investigate suspected Double-Checked Locking
Date Mon, 08 May 2017 13:57:04 GMT
Jiri Danek created ARTEMIS-1152:
-----------------------------------

             Summary: 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


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)

Mime
View raw message