Community
Participate
Working Groups
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.
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.
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.
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).
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.
Checked the patch into trunk (rev.9212).
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
