Description Esteban DUGUEPEROUX 2013-05-24 09:15:32 EDT

Hi,

In our application, on which we use CDO, we begin to use IProgressMonitor, especially with CDOTransaction.commit(IProgressMonitor) to commit changes to CDO server. Sometimes we call the following code snippet :

try {
new ProgressMonitorDialog(PlatformUI.getWorkbench().getDisplay().getActiveShell()).run(true, true, new IRunnableWithProgress() {

     @Override
     public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
         try {
        cdoTransaction.commit(monitor);
     } catch (CommitException e) {
        e.printStackTrace();
         }
     }
});
} catch (InvocationTargetException e) {
    e.printStackTrace();
} catch (InterruptedException e) {
    e.printStackTrace();
}
}

with that the CDO server notify correctly the progress monitor on the client through org.eclipse.net4j.signal.RequestWithMonitoring$1 in another thread that the thread one which run the IRunnableWithProgress.

But in the case of saving through a IEditorPart, it is a Saveable which is used. Concretely it is org.eclipse.ui.Saveable.doSave(org.eclipse.core.runtime.IProgressMonitor) which is called by org.eclipse.jface.window.ApplicationWindow.run() with a org.eclipse.ui.internal.dialogs.EventLoopProgressMonitor. Like we have already a IProgressMonitor offered with Saveable.doSave(IProgressMonitor) then we call CDOTransaction.commit(IProgressMonitor) with this IProgressMonitor. But this EventLoopProgressMonitor when called by RequestWithMonitoring$ in a non UI thread, it throws a "org.eclipse.swt.SWTException: Invalid thread access" because of call to EventLoopProgressMonitor.runEventLoop() call.

The stacktrace :

org.eclipse.swt.SWTException: Invalid thread access
	at org.eclipse.swt.SWT.error(SWT.java:4083)
	at org.eclipse.swt.SWT.error(SWT.java:3998)
	at org.eclipse.swt.SWT.error(SWT.java:3969)
	at org.eclipse.swt.widgets.Display.error(Display.java:1249)
	at org.eclipse.swt.widgets.Display.checkDevice(Display.java:755)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3646)
	at org.eclipse.ui.internal.dialogs.EventLoopProgressMonitor.runEventLoop(EventLoopProgressMonitor.java:123)
	at org.eclipse.ui.internal.dialogs.EventLoopProgressMonitor.isCanceled(EventLoopProgressMonitor.java:97)
	at org.eclipse.core.runtime.ProgressMonitorWrapper.isCanceled(ProgressMonitorWrapper.java:106)
	at org.eclipse.core.runtime.SubMonitor$RootInfo.isCanceled(SubMonitor.java:259)
	at org.eclipse.core.runtime.SubMonitor.isCanceled(SubMonitor.java:516)
	at org.eclipse.core.runtime.ProgressMonitorWrapper.isCanceled(ProgressMonitorWrapper.java:106)
	at org.eclipse.core.runtime.ProgressMonitorWrapper.isCanceled(ProgressMonitorWrapper.java:106)
	at org.eclipse.core.runtime.ProgressMonitorWrapper.isCanceled(ProgressMonitorWrapper.java:106)
	at org.eclipse.core.runtime.ProgressMonitorWrapper.isCanceled(ProgressMonitorWrapper.java:106)
	at org.eclipse.net4j.util.om.monitor.EclipseMonitor.isCanceled(EclipseMonitor.java:53)
	at org.eclipse.net4j.signal.RequestWithMonitoring$1.run(RequestWithMonitoring.java:145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at java.lang.Thread.run(Unknown Source)


IProgressMonitor API does not specify if it can be used by several threads. Then I think we can say that there is a bug in EventLoopProgressMonitor which does not check if we are in the UI tread before calling EventLoopProgressMonitor.runEventLoop(). In addition CDO could ensure that IProgressMonitor.isCanceled()/worked() is called in the same thread as the one calling CDOTransaction.commit(IProgressMonitor).


See also http://www.eclipse.org/forums/index.php/t/487683/

https://bugs.eclipse.org/bugs/show_bug.cgi?id=408319 has been raised on Platform UI component but has been closed has wontfix because we considers that IProgressMonitor.isCanceled()/worked() must be called by the same thread that has called IProgressMonitor.beginTask().

Then I raise this bugzilla to track this issue.