Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0, 2.7
    • Component/s: lang.exception.*
    • Labels:
      None

      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.

      1. LANG-369.patch
        4 kB
        Henri Yandell
      2. LANG-369-2.patch
        3 kB
        Sebb

        Issue Links

          Activity

          Hide
          Sebb added a comment -

          Applied patch:

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

          Show
          Sebb added a comment - Applied patch: URL: http://svn.apache.org/viewvc?rev=755391&view=rev Log: LANG-369 - must use fixed object as lock target
          Hide
          Sebb added a comment -

          Patch which adds static final Object as a lock.

          Show
          Sebb added a comment - Patch which adds static final Object as a lock.
          Hide
          Sebb added a comment -

          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

          Show
          Sebb added a comment - 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
          Hide
          Henri Yandell added a comment -

          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.

          Show
          Henri Yandell added a comment - 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.
          Hide
          Ben Speakmon added a comment -

          Patch looks safe.

          Show
          Ben Speakmon added a comment - Patch looks safe.
          Hide
          Henri Yandell added a comment -

          Patch that liberally splashes synchronized around the variable.

          Show
          Henri Yandell added a comment - Patch that liberally splashes synchronized around the variable.
          Hide
          Henri Yandell added a comment -

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

          Show
          Henri Yandell added a comment - Agreed - synchronizing the variable seems the best thing to do.

            People

            • Assignee:
              Unassigned
              Reporter:
              Sebb
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development