Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 270139 - NPE when attempting to lock an already locked object from a second client
Summary: NPE when attempting to lock an already locked object from a second client
Status: CLOSED FIXED
Alias: None
Product: EMF
Classification: Modeling
Component: cdo.core (show other bugs)
Version: 2.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: M6   Edit
Assignee: Simon Mc Duff CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-26 12:04 EDT by Stephane fournier CLA
Modified: 2010-06-29 09:21 EDT (History)
2 users (show)

See Also:
stepper: galileo+
stepper: review+


Attachments
Patch v1 (4.21 KB, patch)
2009-03-30 21:17 EDT, Simon Mc Duff CLA
no flags Details | Diff
Patch v2 (4.21 KB, patch)
2009-03-30 21:20 EDT, Simon Mc Duff CLA
no flags Details | Diff
Patch v3 (4.39 KB, patch)
2009-03-31 01:55 EDT, Eike Stepper CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephane fournier CLA 2009-03-26 12:04:23 EDT
Build ID: I20090313-0100
CDO and NET4J : 2.0.0M6

Steps To Reproduce:
1.Start an Eclipse SDK open a CDO Session, transaction, a resource...
2. Select an object, let's say "dummy object"
3. Right click, and lock it for instance writeLock.
4. Reproduce the same scenario on a second client connected to the same resource. 

When attempting to lock the object "dummy object" from this second client, a NPE is thrown.

More information:

The Lock operation is run through a LongRunningAction.
This later one delegates to a Job.

LongRunningAction.getShell() (to report the error message) is called from a NON UI thread.
Hence, PlatformUI.getWorkbench().getActiveWorkbenchWindow() in LongRunningAction.getWorkbenchWindow() returns null and getShell() does not check that.

To Fix that, I replace the Job by an UIJob in LongRunningAction.sageRun() that ensures to be in the UI thread.

Other fixes are possible, I let you see the better way to fix that...
Comment 1 Simon Mc Duff CLA 2009-03-26 15:59:01 EDT
Can you provide the stack trace please ?

Simon
Comment 2 Stephane fournier CLA 2009-03-27 04:10:35 EDT
Hi Simon,

Here is the stack trace :
java.lang.NullPointerException
	at org.eclipse.net4j.util.ui.actions.LongRunningAction.getShell(LongRunningAction.java:132)
	at org.eclipse.emf.cdo.internal.ui.actions.AbstractLockObjectsAction.doRun(AbstractLockObjectsAction.java:86)
	at org.eclipse.net4j.util.ui.actions.LongRunningAction$1.run(LongRunningAction.java:164)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)

Have a look at PlatformUI.getWorkbench().getActiveWorkbenchWindow() javadoc.
It is clearly explained that this method returns null if there is no active workbench window or if called from a non-UI thread.

In the other CDO UI actions, most of the time, LongRunningAction.getShell() is called in the preRun() method called from the UI Thread.
But when LongRunningAction.getShell() is called to report to the end-user a message, the call is done within the Job.run(IProgressMonitor progressMonitor) which is in its own worker thread.

The actions where this NPE can occur are :
a) AbstractLockObjectsAction.doRun()
b) OpenSessionAction.doRun()
c) RegisterPackagesAction.doRun()

Stephane.
Comment 3 Simon Mc Duff CLA 2009-03-30 21:17:35 EDT
Created attachment 130333 [details]
Patch v1
Comment 4 Simon Mc Duff CLA 2009-03-30 21:18:19 EDT
Vic or Eike could review it.

The problem was in the LongRunningAction. I added a getDisplay().

Simon
Comment 5 Simon Mc Duff CLA 2009-03-30 21:20:09 EDT
Created attachment 130334 [details]
Patch v2

Forgot to change syncExec to asyncExec! :-)
Comment 6 Eike Stepper CLA 2009-03-31 01:55:32 EDT
Created attachment 130349 [details]
Patch v3

Ready to be committed.
Comment 7 Simon Mc Duff CLA 2009-03-31 16:35:15 EDT
Committed to HEAD
Comment 8 Eike Stepper CLA 2009-04-02 17:35:00 EDT
Fix available in EMF CDO 2.0.0M6b
Comment 9 Eike Stepper CLA 2009-06-27 11:49:51 EDT
Generally available.