[#DERBY-3308] Broken synchronization for event handling in ClientPooledConnection40

Derby

Broken synchronization for event handling in ClientPooledConnection40

Created: 09/Jan/08 01:15 PM Updated: 11/Feb/08 01:28 PM

Return to search

Component/s:

JDBC, Network Client

Affects Version/s:

10.2.2.0, 10.3.2.1, 10.4.1.3

Fix Version/s:

10.2.2.1, 10.3.3.0, 10.4.1.3

Time Tracking:

Issue & Sub-Tasks

Issue Only

Not Specified

File Attachments:

File Name	Date Attached	Attached By	Size
derby-3308-1a-eventlisteners_synchronization.diff	2008-01-23 12:30 PM	Kristian Waagan	4 kB
derby-3308-1b-eventlisteners_synchronization.diff	2008-01-23 01:58 PM	Kristian Waagan	4 kB

Resolution Date:

23/Jan/08 02:13 PM

Sub-Tasks:

All

Open

No sub-tasks match this view.

Description

« Hide

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).

All

Comments

Work Log

Change History

Subversion Commits

Sort Order:

Kristian Waagan made changes - 09/Jan/08 01:15 PM

Field	Original Value	New Value
Component/s		Network Client [ 11690 ]

Kristian Waagan made changes - 23/Jan/08 12:19 PM

Status

Open [ 1 ]

In Progress [ 3 ]

[ Permlink | « Hide ]

Kristian Waagan added a comment - 23/Jan/08 12:30 PM

'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.

Kristian Waagan made changes - 23/Jan/08 12:30 PM

Attachment

derby-3308-1a-eventlisteners_synchronization.diff [ 12373821 ]

Kristian Waagan made changes - 23/Jan/08 12:31 PM

Derby Info		[Patch Available]
Fix Version/s		10.4.0.0 [ 12312540 ]

[ Permlink | « Hide ]

Knut Anders Hatlen added a comment - 23/Jan/08 01:33 PM

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.)

[ Permlink | « Hide ]

Kristian Waagan added a comment - 23/Jan/08 01:58 PM

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.

Kristian Waagan made changes - 23/Jan/08 01:58 PM

Attachment

derby-3308-1b-eventlisteners_synchronization.diff [ 12373828 ]

Repository	Revision	Date	User	Message
ASF	#614536	Wed Jan 23 14:12:02 UTC 2008	kristwaa	~~DERBY-3308~~: Broken synchronization for event handling in ClientPooledConnection40. Patch file: ~~DERBY-3308~~-1b-eventlisteners_synchronization.diff
				Files Changed
				MODIFY /db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java

[ Permlink | « Hide ]

Kristian Waagan added a comment - 23/Jan/08 02:13 PM - 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.

Kristian Waagan made changes - 23/Jan/08 02:13 PM

Resolution		Fixed [ 1 ]
Derby Info	[Patch Available]
Status	In Progress [ 3 ]	Resolved [ 5 ]

Repository	Revision	Date	User	Message
ASF	#618639	Tue Feb 05 13:15:55 UTC 2008	kristwaa	~~DERBY-3308~~: Broken synchronization for event handling in ClientPooledConnection40. Merged revision 614536 from trunk (10.4).
				Files Changed
				MODIFY /db/derby/code/branches/10.2/java/client/org/apache/derby/client/ClientPooledConnection40.java

Repository	Revision	Date	User	Message
ASF	#618640	Tue Feb 05 13:15:56 UTC 2008	kristwaa	~~DERBY-3308~~: Broken synchronization for event handling in ClientPooledConnection40. Merged revision 614536 from trunk (10.4).
				Files Changed
				MODIFY /db/derby/code/branches/10.3/java/client/org/apache/derby/client/ClientPooledConnection40.java

[ Permlink | « Hide ]

Kristian Waagan added a comment - 05/Feb/08 01:19 PM

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.

Kristian Waagan made changes - 05/Feb/08 01:19 PM

Fix Version/s		10.3.2.2 [ 12312885 ]
Fix Version/s		10.2.2.1 [ 12312251 ]

[ Permlink | « Hide ]

Kristian Waagan added a comment - 11/Feb/08 01:28 PM

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

Kristian Waagan made changes - 11/Feb/08 01:28 PM

Status

Resolved [ 5 ]

Closed [ 6 ]