Commons Dbcp
  1. Commons Dbcp
  2. DBCP-271

Thread safety issues in AbandonedTrace class

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Labels:
      None

      Description

      DBCP-270 implies that the AbandonedTrace class is called from multiple threads, however it is not thread-safe:

      • SimpleDateFormat is not threadsafe, but it is called without protection.
      • various instance fields need to be made final to ensure that they are published correctly (memory visibility)
      • getLastUsed/setLastUsed need to be synchronized - or better, createdBy made volatile so printStackTrace can access the variable safely
        (createdBy is rewritten, not updated, so lost updates are not important)

      Also, the Javadoc for printStackTrace() is wrong.

        Activity

        Sebb created issue -
        Sebb made changes -
        Field Original Value New Value
        Description DBCP-270 implies that the AbandonedTrace class is called from multiple threads, however it is not thread-safe:

        * SimpleDateFormat is not threadsafe, but it is called without protection.
        * various instance fields need to be made final to ensure that they are published correctly (memory visibility)
        * getLastUsed/setLastUsed need to be synchronized

        Also, the Javadoc for printStackTrace() is wrong.
        DBCP-270 implies that the AbandonedTrace class is called from multiple threads, however it is not thread-safe:

        * SimpleDateFormat is not threadsafe, but it is called without protection.
        * various instance fields need to be made final to ensure that they are published correctly (memory visibility)
        * getLastUsed/setLastUsed need to be synchronized - or better, createdBy made volatile so printStackTrace can access the variable safely
        (createdBy is rewritten, not updated, so lost updates are not important)

        Also, the Javadoc for printStackTrace() is wrong.
        Hide
        Sebb added a comment -

        Also, as far as I can tell, the parent instance field is used but never initialised.

        Show
        Sebb added a comment - Also, as far as I can tell, the parent instance field is used but never initialised.
        Hide
        Phil Steitz added a comment -

        Made relevant instance fields volatile or final in r737478.

        Show
        Phil Steitz added a comment - Made relevant instance fields volatile or final in r737478.
        Phil Steitz made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Phil Steitz made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development