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-2.patch
        3 kB
        Sebb
      2. LANG-369.patch
        4 kB
        Henri Yandell

        Issue Links

          Activity

          Sebb created issue -
          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.
          Henri Yandell made changes -
          Field Original Value New Value
          Fix Version/s 2.4 [ 12312481 ]
          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.
          Henri Yandell made changes -
          Attachment LANG-369.patch [ 12369216 ]
          Hide
          Ben Speakmon added a comment -

          Patch looks safe.

          Show
          Ben Speakmon added a comment - Patch looks safe.
          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.
          Henri Yandell made changes -
          Status Open [ 1 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          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
          Sebb made changes -
          Resolution Fixed [ 1 ]
          Status Closed [ 6 ] Reopened [ 4 ]
          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.
          Sebb made changes -
          Attachment LANG-369-2.patch [ 12402204 ]
          Henri Yandell made changes -
          Fix Version/s 3.0 [ 12311714 ]
          Fix Version/s 2.x [ 12313695 ]
          Fix Version/s 2.4 [ 12312481 ]
          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
          Sebb made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Henri Yandell made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Henri Yandell made changes -
          Component/s lang.exception.* [ 12313201 ]
          Niall Pemberton made changes -
          Link This issue relates to LANG-584 [ LANG-584 ]
          Mark Thomas made changes -
          Workflow jira [ 12416103 ] Default workflow, editable Closed status [ 12602216 ]
          Henri Yandell made changes -
          Fix Version/s 2.7 [ 12321644 ]
          Fix Version/s 2.x [ 12313695 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Closed Closed
          13d 2h 22m 1 Henri Yandell 12/Nov/07 19:58
          Closed Closed Reopened Reopened
          487d 18h 35m 1 Sebb 14/Mar/09 14:33
          Reopened Reopened Resolved Resolved
          3d 6h 22m 1 Sebb 17/Mar/09 20:55
          Resolved Resolved Closed Closed
          59d 10h 36m 1 Henri Yandell 16/May/09 07:32

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development