Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 341261

Summary: User should be able to add descriptor to connected session while threads doing CRUD operations concurrently.
Product: z_Archived Reporter: Andrei Ilitchev <andrei.ilitchev>
Component: EclipselinkAssignee: Andrei Ilitchev <andrei.ilitchev>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: michael.f.obrien
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
suggested patch
none
updated patch
none
patch - take 3
none
The patch to be checked in none

Description Andrei Ilitchev CLA 2011-03-29 13:12:09 EDT 
Adding a new descriptor to connected session while other threads doing CRUD operation concurrently may cause problems because:

1. Both addDescriptor and addDescriptors methods end up disconnecting, then reconnecting sequencing.
While sequencing has been disconnected:
a) if a lock has been acquired by SequencingManager when it still was connected then it's never released (reported by Stephen McRitchie);
b) attempt to assign sequence number by a concurrent thread would cause NPE.

2. Project.descriptors Map is a HashMap.
Comment in HashMap class says:
"If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally."
Neither adding  descriptor nor getting descriptor is externally synchronized,
therefore adding a new descriptor to descriptors map may cause session.getDescriptor() to fail - and it's used everywhere.
Comment 1 Andrei Ilitchev CLA 2011-03-30 17:23:18 EDT 
Created attachment 192228 [details]
suggested patch

The patch should address all problems listed in the bug.

Defined a new method onAddDescriptors on SequencingManager class that should be called when descriptors are added to the project. The new method connects the sequences used by passed descriptors without disconnecting sequencing;
No longer search locks map for locks to be released - the acquired lock is kept as a local variable in the calling methods.
Altered addDescriptor and addDescriptors methods to allow concurrent read access to descriptors in case the session is connected.

No test added, but addDescriptors method used all over core LRG tests, which pass.
Comment 2 Andrei Ilitchev CLA 2011-04-01 17:37:45 EDT 
Created attachment 192394 [details]
updated patch

Added two multithreaded tests: in both the main thread adds new descriptors with either default, or native, or table sequences; while several concurrent tests in one case preallocate sequence numbers; in another insert objects:
tests.simultaneous.AddDescriptorsMultithreadedTest;

Updated AbstractTransactionController so that adding descriptors never cause calling clearSequencingListeners method that may derail concurrently committed transaction;

In DatasourcePlatform change sequences map from HashMap to ConcurrentHashMap. Also got rid of iterating directly on sequences map - now clone first then iterate the clone.

Cloning of Login done on several occasions (ConnectionPool.buildConnection, in ConnectionPolicy, in SequencingManager) but none of these cases interested in cloned Platform to be underneath the login.
Cloning of Platform cause deep cloning of sequences - each one is recreated on the clone platform.
Therefore introduced a new method Login.shalowClone and used it in all three places in core code where Login.clone was used.
Comment 3 Andrei Ilitchev CLA 2011-04-04 17:57:11 EDT 
Created attachment 192509 [details]
patch - take 3

- added addSequence method to DatabaseSession so that sequences could be added to connected session in  a safe way (by copying the whole map, adding the sequence, then overriding the original map - just like in descriptors case);

- BatchWritingMechanism was changed to hold its maxSize; maxBatchWritingSize attribute DatabasePlatform now initialized to 0; if not set by the user then default value is used (different for each subclass);

- added comments to Platform and Login that addSequences not to be used while session is connected;

- removed shalowClone method (that I introduced in the prev. version of the patch) from DatasourceLogin. Sequencemanager and ConnectionPool now don't clone the login that used for SequenceConnectionPool and built accessor respectively;

- introduced sequencesLock and descriptorsLock to synchronized access to methods that add / remove sequences on DatasourcePlatform and methods that add descriptors in Project class (except Project.adDescriptor that doesn't take session as a a prameter).
Comment 4 Andrei Ilitchev CLA 2011-04-05 18:40:40 EDT 
Created attachment 192602 [details]
The patch to be checked in

Several small fixes.
Added more testing: 
CannotOverrideConnectedSequenceTest to FeatureTestModel - SequenceTestSuite;
More scenarious tested with AddDescriptorsMultithreadedTest in SimultaneousTestModel.
Comment 5 Andrei Ilitchev CLA 2011-04-05 18:58:43 EDT 
Checked the patch into trunk (rev.9212).
Comment 6 Eclipse Webmaster CLA 2022-06-09 10:04:06 EDT 
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
Comment 7 Eclipse Webmaster CLA 2022-06-09 10:07:11 EDT 
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink