ExceptionUtils not thread-safe

• Key: LANG-369
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Sebb
• Fix Version/s: 3.0, 2.x

Description

The ExceptionUtils class does not appear to be thread-safe:

• CAUSE_METHOD_NAMES is not final, so may not be visible to all threads
• addCauseMethodName() and removeCauseMethodName() can update CAUSE_METHOD_NAMES but are not synch.
• all accesses to CAUSE_METHOD_NAMES probably need to be synchronised

The documentation does not state whether or not the class is thread-safe, but given that it only has static methods it does not make any sense unless it is thread-safe.

Comments

[Henri Yandell]

Agreed - synchronizing the variable seems the best thing to do.

[Henri Yandell]

Patch that liberally splashes synchronized around the variable.

[Ben Speakmon]

Patch looks safe.

[Henri Yandell]

svn ci -m "Applying the synchronization from LANG-369" src/java/org/apache/commons/lang/exception/ExceptionUtils.java

Sending src/java/org/apache/commons/lang/exception/ExceptionUtils.java
Transmitting file data .
Committed revision 594278.

[Sebb]

I think the wrong lock object is used in the synchronization.

The code uses the CAUSE_METHOD_NAMES object as the lock and then changes it in the synchronized block, i.e. the lock object is changed.

This occurs in removeCauseMethodName() and addCauseMethodName().

Either lock on the class, or use a dummy static final Object as the lock.

This applies to 2.4 branch and trunk

[Sebb]

Patch which adds static final Object as a lock.

Applied patch:

URL: http://svn.apache.org/viewvc?rev=755391&view=rev
Log:
LANG-369 - must use fixed object as lock target