Home » Eclipse Projects » EclipseLink » ConcurrentModificationException prior to updating entity.
| | | | |
| Re: ConcurrentModificationException prior to updating entity. [message #1695495 is a reply to message #511697] |
Fri, 15 May 2015 10:07  |
 Nuno Godinho de Matos
Messages: 34
Registered: September 2012 |
Member |
|
|
Hi there,
I had a very similar situation under eclipse 3.2.2 version glassfish 3.1.2.2.
java.util.ConcurrentModificationException
at java.util.IdentityHashMap$IdentityHashMapIterator.nextIndex(IdentityHashMap.java:732)
at java.util.IdentityHashMap$KeyIterator.next(IdentityHashMap.java:822)
at org.eclipse.persistence.internal.queries.ContainerPolicy.mergeChanges(ContainerPolicy.java:1077)
at org.eclipse.persistence.mappings.CollectionMapping.mergeChangesIntoObject(CollectionMapping.java:1334)
at org.eclipse.persistence.internal.descriptors.ObjectBuilder.mergeChangesIntoObject(ObjectBuilder.java:3445)
at org.eclipse.persistence.internal.sessions.MergeManager.mergeChangesOfWorkingCopyIntoOriginal(MergeManager.java:746)
at org.eclipse.persistence.internal.sessions.MergeManager.mergeChangesOfWorkingCopyIntoOriginal(MergeManager.java:618)
at org.eclipse.persistence.internal.sessions.MergeManager.mergeChanges(MergeManager.java:267)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.mergeChangesIntoParent(UnitOfWorkImpl.java:3254)
at org.eclipse.persistence.internal.sessions.RepeatableWriteUnitOfWork.mergeChangesIntoParent(RepeatableWriteUnitOfWork.java:366)
at org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.mergeClonesAfterCompletion(UnitOfWorkImpl.java:3386)
at org.eclipse.persistence.transaction.AbstractSynchronizationListener.afterCompletion(AbstractSynchronizationListener.java:213)
at org.eclipse.persistence.transaction.JTASynchronizationListener.afterCompletion(JTASynchronizationListener.java:79)
at com.sun.jts.jta.SynchronizationImpl.after_completion(SynchronizationImpl.java:156)
at com.sun.jts.CosTransactions.RegisteredSyncs.distributeAfter(RegisteredSyncs.java:213)
at com.sun.jts.CosTransactions.TopCoordinator.afterCompletion(TopCoordinator.java:2589)
at com.sun.jts.CosTransactions.CoordinatorTerm.commit(CoordinatorTerm.java:444)
at com.sun.jts.CosTransactions.TerminatorImpl.commit(TerminatorImpl.java:250)
at com.sun.jts.CosTransactions.CurrentImpl.commit(CurrentImpl.java:633)
at com.sun.jts.jta.TransactionManagerImpl.commit(TransactionManagerImpl.java:332)
at com.sun.enterprise.transaction.jts.JavaEETransactionManagerJTSDelegate.commitDistributedTransaction(JavaEETransactionManagerJTSDelegate.java:174)
at com.sun.enterprise.transaction.JavaEETransactionManagerSimplified.commit(JavaEETransactionManagerSimplified.java:861)
at com.sun.ejb.containers.BaseContainer.completeNewTx(BaseContainer.java:5136)
at com.sun.ejb.containers.BaseContainer.postInvokeTx(BaseContainer.java:4901)
at com.sun.ejb.containers.BaseContainer.postInvoke(BaseContainer.java:2045)
at com.sun.ejb.containers.BaseContainer.postInvoke(BaseContainer.java:1994)
at com.sun.ejb.containers.EJBObjectInvocationHandler.invoke(EJBObjectInvocationHandler.java:213)
at com.sun.ejb.containers.EJBObjectInvocationHandlerDelegate.invoke(EJBObjectInvocationHandlerDelegate.java:79)
at com.sun.proxy.$Proxy405.processTelegram(Unknown Source)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at com.sun.corba.ee.impl.presentation.rmi.ReflectiveTie.dispatchToMethod(ReflectiveTie.java:144)
at com.sun.corba.ee.impl.presentation.rmi.ReflectiveTie._invoke(ReflectiveTie.java:174)
at com.sun.corba.ee.impl.protocol.CorbaServerRequestDispatcherImpl.dispatchToServant(CorbaServerRequestDispatcherImpl.java:528)
The exception was not deterministic, which was very surprising since I was not able to find any concurrent modifications taking place at the same time. The test that was having this merge process explosion,
Essentially was running single threaded:
The test would be doing something of the sort.
MyRemoteEjb ejb = GlasfisshClicentUtil.giveMeRemoteEJB();
ejb.reserveJacketOnCloset();
And on the server side you would have a dummy entity such as a Closet entity where you would modify a one-to-many relationship doing something like:
myClosed.getReservedJackets().add(jacketOnClosedThatIsNowReserved);
The commit of the transaciton was blowing up during the merge process of this Closet One-To-Many relationship.
While debuging the code, whenever I did not see the exception happen, I was able to check the following.
The ChangeSetEntity of eclipse link had for this one too many attribute a changeSet with addedEntities.
And the added entity on the change set would apperently be a "clone".
If were to use the eclise link APIs to search for the entity in this added change set on the eclipse link server session cache, I could clearly see:
The entity that is part of the change set is an object id different from the entity that is on the server session cache.
One of them, the one in the changeset has a @Version higher than the on on the server ssion cache, which has a @Version equal to the one I see in the databsae.
However, when the concurrent modification excpetion happened, the blow up was that eclipse link was doing the following:
(1) Create an interator over the entities on the addChangeSet.
(2) Itereation.hasNext() = true
(3) jacket = iterator.next() <--- fetches the clone
(4) runMergePOrocess on jacket
(5) iterator.hasNext() <----- It was not supposed to have a next but some times it would say, yes there were more changed entities
(6) itertort.next() <----- blow up
And in this case, if I had the break point before the call to iterator.next(), I could see that the entity on change set appeared to no longer be a "clone" it was the exact same entity that existed in the server session cache. it looked like it was the original.
The most surprsing part, is that the code that seemed to lead this concurrent modification exception to happen was a eclipse link even listener.
The persistence.xml of the project breaking up hat this in there:
<!--property name="eclipselink.descriptor.customizer.Closet" value="dispatch.listener.StagingClosetStatusChangeListener" /-->
By commenting this eclipse link property the merge process stopped breaking.
But the fact is, the code in this change listener, as far as i could see while debugging it:
It did absolutely nothing.
It looked at the ChangeSet instance - it looked at the things that had changed to see if they were interesting, but it modified nothing.
It simply looked at it and went away.
The code looked perefectly harmless.
Of course - some geters in eclipse link - such as on lazy loaded lists on entiteis, are totally booby trapped, so it is hard to say convincingly - it looks harmless but are you sure about side effects?
Well in this case, I looked like peeking at the change set would - in non detemrinistic mannger - break the merge process of the one-to-many relationshp, and all of this seemed to take place on single thread running on the server soide (which should make the explosion deterministic, but it did not).
I was never able to suceed in puting a breakpoin on the idnetity map from which the iterator that broke actualy being modified, so I was not ever able to discover what eclipse link code exactly triggered the modification of the change set manipulated in the merge process.
I was not satisfied at all with the solution of having to take away the change listener.
I also know that I could have just hacked the problem away by discaring the iterator, and modifying the source coe of eclipse link to loop ofver a an indepent new ArrayList<Entities> , instead of depending on iterator.next() on collection that is being hammered somehow.
So - I have no conclusion on what happened here, but it looked like listening ot changeset events is not the best idea.
This is the method that was exploding:
/**
* INTERNAL:
* Merge changes from the source to the target object. Because this is a
* collection mapping, values are added to or removed from the collection
* based on the change set.
*/
public void mergeChanges(CollectionChangeRecord changeRecord, Object valueOfTarget, boolean shouldMergeCascadeParts, MergeManager mergeManager, AbstractSession targetSession) {
ObjectChangeSet objectChanges;
// Step 1 - iterate over the removed changes and remove them from the container.
Iterator removeObjects = changeRecord.getRemoveObjectList().keySet().iterator();
// Ensure the collection is synchronized while changes are being made,
// clone also synchronizes on collection (does not have cache key read-lock for indirection).
// Must synchronize of the real collection as the clone does so.
Object synchronizedValueOfTarget = valueOfTarget;
if (valueOfTarget instanceof IndirectCollection) {
synchronizedValueOfTarget = ((IndirectCollection)valueOfTarget).getDelegateObject();
}
synchronized (synchronizedValueOfTarget) {
while (removeObjects.hasNext()) {
objectChanges = (ObjectChangeSet) removeObjects.next();
removeFrom(objectChanges.getOldKey(), objectChanges.getTargetVersionOfSourceObject(mergeManager, targetSession), valueOfTarget, targetSession);
if (!mergeManager.shouldMergeChangesIntoDistributedCache()) {
mergeManager.registerRemovedNewObjectIfRequired(objectChanges.getUnitOfWorkClone());
}
}
// Step 2 - iterate over the added changes and add them to the container.
Iterator addObjects = changeRecord.getAddObjectList().keySet().iterator();
while (addObjects.hasNext()) {
objectChanges = (ObjectChangeSet) addObjects.next(); <----------------------- Right here . But it was simply not possible to figure out who modified changeRecord.getAddObjectList() or why
Object object = null;
if (shouldMergeCascadeParts) {
object = mergeCascadeParts(objectChanges, mergeManager, targetSession);
}
if (object == null) {
// Retrieve the object to be added to the collection.
object = objectChanges.getTargetVersionOfSourceObject(mergeManager, targetSession, false);
}
// I am assuming that at this point the above merge will have created a new object if required
if (mergeManager.shouldMergeChangesIntoDistributedCache()) {
//bug#4458089 and 4454532- check if collection contains new item before adding during merge into distributed cache
if (!contains(object, valueOfTarget, mergeManager.getSession())) {
addInto(objectChanges.getNewKey(), object, valueOfTarget, mergeManager.getSession());
}
} else {
addInto(objectChanges.getNewKey(), object, valueOfTarget, mergeManager.getSession());
}
}
}
}
By the way, one final remark.
The line comments that you guys put on the code are great! They do help a lot.
However, and please note, I am well aware I am saying this about an old version of code of eclipse link and its the newer versions the comment might be unfair, but Seriously:
Please, do improve the javadoc of the private methods.
The logic that is being executed by eclipse link is for sure non trivial most of the time, and the only helpful documentaiton that there is on the code is to be found are the sporadic line comments placed in the code. Going into an arbitrary method of eclipse-link and understanding the code, is like going to the midle of the desert hoping to find a drop of water.
I would say that at the very least, the parameters of these methods should be properly document with @param javadoc.
Such as @param changeRecord
Instance encapsulating the changes done within the context of a transaction on an attribute of a JPA instance close, such as, for example, changes done on a One-To-Many attribute.
Then Ok! I go into the method and I know what you guys are talking about. What information is in the instance variable. What granularity of information is in there. And such...
And also, it would be quite helpful when we are dealing for exmaple with methods that deal with merging:
That the variables have much clearer names, that specify, for example:
"hey this is a clone and is suppose dot get its changes copied over to an original in the server session cache"
"this thing is an original - it reflects the current state of the database - it comes from the global manage eclipse session - and it just about to get the updates of clone to ensure that the server session cache does not become stale in relation to the databsae state ", etc...
It simply has to be less painful to go into a method and know what data is being manipulated.
When you call a variable something like "object", quite frankly, it's just useless variable name. It does not take a lot of effort to come up with something a bit more meaningful.
While I can understand being somewhat lazy on comments on trivial code, you guys are writing non trivial code. So please improve the self documentation of the code, so that we can do a better job of help ing ourselves when we have issues with the framework.
Kindest regards.
[Updated on: Fri, 15 May 2015 10:34] Report message to a moderator |
|
|
Goto Forum:
Current Time: Wed Oct 09 22:49:05 GMT 2024
Powered by FUDForum. Page generated in 0.06946 seconds |
] 
Search
Help
Register
Login
Home
