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

Bug 346477

Summary: Detached revision not found for commit notification
Product: [Modeling] EMF Reporter: Egidijus Vaisnora <vaisegid>
Component: cdo.coreAssignee: Egidijus Vaisnora <vaisegid>
Status: CLOSED FIXED QA Contact: Eike Stepper <stepper>
Severity: normal    
Priority: P3 Flags: stepper: review-
Version: 4.0   
Target Milestone: ---   
Hardware: Macintosh   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch v1
none
Patch v2
none
Patch v3 none

Description Egidijus Vaisnora CLA 2011-05-19 11:23:24 EDT 
As description cold server this stack trace:
java.lang.NullPointerException
	at org.eclipse.emf.cdo.internal.common.protocol.CDODataOutputImpl.writeCDOIDAndVersion(CDODataOutputImpl.java:263)
	at org.eclipse.emf.cdo.internal.common.protocol.CDODataOutputImpl.writeCDOChangeSetData(CDODataOutputImpl.java:184)
	at org.eclipse.emf.cdo.internal.common.protocol.CDODataOutputImpl.writeCDOCommitData(CDODataOutputImpl.java:197)
	at org.eclipse.emf.cdo.internal.common.protocol.CDODataOutputImpl.writeCDOCommitInfo(CDODataOutputImpl.java:212)
	at org.eclipse.emf.cdo.server.internal.net4j.protocol.CommitNotificationRequest.requesting(CommitNotificationRequest.java:37)
	at org.eclipse.emf.cdo.server.internal.net4j.protocol.CDOServerRequest.requesting(CDOServerRequest.java:49)
	at org.eclipse.net4j.signal.Request.doExtendedOutput(Request.java:65)
	at org.eclipse.net4j.signal.Signal.doOutput(Signal.java:296)
	at org.eclipse.net4j.signal.Request.doExecute(Request.java:57)
	at org.eclipse.net4j.signal.SignalActor.execute(SignalActor.java:51)
	at org.eclipse.net4j.signal.Signal.runSync(Signal.java:251)
	at org.eclipse.net4j.signal.SignalProtocol.startSignal(SignalProtocol.java:396)
	at org.eclipse.net4j.signal.Request.sendAsync(Request.java:51)
	at org.eclipse.emf.cdo.server.internal.net4j.protocol.CDOServerProtocol.sendCommitNotification(CDOServerProtocol.java:125)
	at org.eclipse.emf.cdo.internal.server.Session.sendCommitNotification(Session.java:378)
	at org.eclipse.emf.cdo.internal.server.SessionManager.sendCommitNotification(SessionManager.java:299)
	at org.eclipse.emf.cdo.internal.server.Repository.sendCommitNotification(Repository.java:817)
	at org.eclipse.emf.cdo.internal.server.TransactionCommitContext.postCommit(TransactionCommitContext.java:528)
	at org.eclipse.emf.cdo.server.internal.net4j.protocol.CommitTransactionIndication.responding(CommitTransactionIndication.java:263)
	at org.eclipse.emf.cdo.server.internal.net4j.protocol.CDOServerIndicationWithMonitoring.responding(CDOServerIndicationWithMonitoring.java:170)
	at org.eclipse.net4j.signal.IndicationWithMonitoring.responding(IndicationWithMonitoring.java:90)
	at org.eclipse.net4j.signal.IndicationWithResponse.doExtendedOutput(IndicationWithResponse.java:96)
	at org.eclipse.net4j.signal.Signal.doOutput(Signal.java:296)
	at org.eclipse.net4j.signal.IndicationWithResponse.execute(IndicationWithResponse.java:65)
	at org.eclipse.net4j.signal.IndicationWithMonitoring.execute(IndicationWithMonitoring.java:63)
	at org.eclipse.net4j.signal.Signal.runSync(Signal.java:251)
	at org.eclipse.net4j.signal.Signal.run(Signal.java:147)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
	at java.lang.Thread.run(Thread.java:680)
Comment 1 Egidijus Vaisnora CLA 2011-05-19 11:33:42 EDT 
Created attachment 196129 [details]
Patch v1

After investigating code, I found line which IMO could cause this issue. Code relies, that revision will be in the cache, but cache is not responsible to provide all revisions and could be cleaned in every time.  
At least after patching I don't get exception any more. 
I left fetching revision from cache first, but maybe I we need to remove this code.
Comment 2 Egidijus Vaisnora CLA 2011-05-19 11:34:45 EDT 
After investigating, I found code, which seems could cause this issue
Comment 3 Eike Stepper CLA 2011-05-20 06:52:25 EDT 
Created attachment 196200 [details]
Patch v2

I tried to consolidate the revision acquisition but I fear some tests are now failing. That may be related with revman.getRevision() returning null instead of throwing an exception if the revision could not be found. My patch makes an extra check.

Why should null be allowed there? At least it matches the behaviour of the cache that was used formerly. But also that surprises me...
Comment 4 Eike Stepper CLA 2011-05-23 11:44:02 EDT 
Created attachment 196354 [details]
Patch v3
Comment 5 Eike Stepper CLA 2011-05-23 11:45:26 EDT 
Patch v1 and v2 fix the NPE at the wrong place, see v3 for better fix.
Comment 6 Eike Stepper CLA 2011-05-23 11:45:53 EDT 
Committed revision 7821:
- trunk/plugins/org.eclipse.emf.cdo.server
Comment 7 Eike Stepper CLA 2011-05-23 11:46:08 EDT 
Resolved
Comment 8 Eike Stepper CLA 2011-06-23 03:41:02 EDT 
Available in R20110608-1407