Derby
  1. Derby
  2. DERBY-3308

Broken synchronization for event handling in ClientPooledConnection40

    Details

      Description

      Access to the the list of event listeners is not synchronized properly.

      Taken the rather infrequent use of the relevant methods and the small critical sections, I mean it is sufficient to add synchronization to all the methods that access it at the method level. The same approach is taken in ClientPooledConnection (although not followed through consistently, some of the methods are unsynchronized).

        Activity

        Hide
        Kristian Waagan added a comment -

        'derby-3308-1a-eventlisteners_synchronization.diff' synchronizes all methods accessing the list (a java.util.Vector) of statement event listeners.

        I ran derbyall/suites.All without errors. Patch ready for review.

        Show
        Kristian Waagan added a comment - 'derby-3308-1a-eventlisteners_synchronization.diff' synchronizes all methods accessing the list (a java.util.Vector) of statement event listeners. I ran derbyall/suites.All without errors. Patch ready for review.
        Hide
        Knut Anders Hatlen added a comment -

        The patch looks fine to me.

        Since the list is now guarded by synchronization on the pooled connection, couldn't it be changed from Vector to ArrayList? (Should be safe to do with listeners_ in the parent class as well.)

        Show
        Knut Anders Hatlen added a comment - The patch looks fine to me. Since the list is now guarded by synchronization on the pooled connection, couldn't it be changed from Vector to ArrayList? (Should be safe to do with listeners_ in the parent class as well.)
        Hide
        Kristian Waagan added a comment -

        Thanks for commenting on the patch Knut Anders.

        I have incorporated you suggestion about replacing Vector with ArrayList in revision 1b.

        Regarding the suggested changes in the parent class, there are some methods that are synchronized and some that are not. A patch could be attached to the subtask of this issue. Feel free to add it, or maybe I will if I get around to it.

        Show
        Kristian Waagan added a comment - Thanks for commenting on the patch Knut Anders. I have incorporated you suggestion about replacing Vector with ArrayList in revision 1b. Regarding the suggested changes in the parent class, there are some methods that are synchronized and some that are not. A patch could be attached to the subtask of this issue. Feel free to add it, or maybe I will if I get around to it.
        Hide
        Kristian Waagan added a comment - - edited

        Committed 'derby-3308-1b-eventlisteners_synchronization.diff' to trunk with revision 614536.

        I don't expect there will be more work on this issue, will close in a few days.

        Show
        Kristian Waagan added a comment - - edited Committed 'derby-3308-1b-eventlisteners_synchronization.diff' to trunk with revision 614536. I don't expect there will be more work on this issue, will close in a few days.
        Hide
        Kristian Waagan added a comment -

        Backported the fix (revision 614536):
        10.2 -> revision 618639
        10.3 -> revision 618640

        I ran all tests for both branches without errors, and both merges were clean.
        Updated the fix versions accordingly.

        Show
        Kristian Waagan added a comment - Backported the fix (revision 614536): 10.2 -> revision 618639 10.3 -> revision 618640 I ran all tests for both branches without errors, and both merges were clean. Updated the fix versions accordingly.
        Hide
        Kristian Waagan added a comment -

        Haven't seen any problems with the patch in the regression tests. Closing issue.

        Show
        Kristian Waagan added a comment - Haven't seen any problems with the patch in the regression tests. Closing issue.

          People

          • Assignee:
            Kristian Waagan
            Reporter:
            Kristian Waagan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development