Community
Participate
Working Groups
Running MAT on RAP means running in a multi-user environment. As MAT is heavily using background jobs, we need to clearify to which session the job belongs when it needs to update the UI. Running a Job in a server environment means that possibly every session could have started this job. When updating the UI trough a sync/asyncExec, the display of the user session is needed. The default pattern is to use PlatformUI.getWorkbench().getDisplay but this will not work on RAP in many sitations as RAP cannot decide which display of which session to use. Fixing this is pretty easy as most of the jobs have a reference to a widget lying around and we can just ask the widget for the display. No semantics to SWT are changed that way but this will fix the multi-user case. If there are no objections. I'd provide a patch for this.
I would welcome a patch for this. The uses of PlatformUI.getWorkbench().getDisplay() are quite extensive, so a patch will save a lot of time working out how to fix each one. Do any other uses of PlatformUI.getWorkbench().getDisplay() other than for asyncExec or syncExec need to be fixed? Does the use in InspectorView.clearInput need to be fixed? What happens with RAP if a use isn't fixed?
Using a widget to get the display is straightforward. Inside a Job.doRun method is it safe to call getSite().getShell().getDisplay() as will the Job be run on a UI thread? From the doc: Shell org.eclipse.ui.IWorkbenchSite.getShell() Returns the shell for this workbench site. Not intended to be called from outside the UI thread. Clients should call IWorkbench.getDisplay() to gain access to the display rather than calling getShell().getDisplay(). For compatibility, this method will not throw an exception if called from outside the UI thread, but the returned Shell may be wrong. Should we extract the display outside of the Job? Extracting a display inside a standard MAT query seems to be hard (e.g. CopyActions), as the snapshot seems to be the only object available. Do we need a new argument type to get the display or some other UI component?
The symptoms of not fixing this are exceptions such as: java.lang.IllegalStateException: No context available outside of the request service lifecycle. at org.eclipse.rwt.internal.service.ContextProvider.getContext(ContextProvider.java:107) at org.eclipse.rwt.internal.service.ContextProvider.getStateInfo(ContextProvider.java:160) at org.eclipse.rwt.SessionSingletonBase.getInstance(SessionSingletonBase.java:84) at org.eclipse.ui.internal.Workbench.getInstance(Workbench.java:378) at org.eclipse.ui.PlatformUI.getWorkbench(PlatformUI.java:94)
(In reply to comment #2) > Using a widget to get the display is straightforward. > > Inside a Job.doRun method is it safe to call getSite().getShell().getDisplay() > as will the Job be run on a UI thread? > > From the doc: > > Shell org.eclipse.ui.IWorkbenchSite.getShell() > > Returns the shell for this workbench site. Not intended to be called from > outside the UI thread. Clients should call IWorkbench.getDisplay() to gain > access to the display rather than calling getShell().getDisplay(). > > For compatibility, this method will not throw an exception if called from > outside the UI thread, but the returned Shell may be wrong. > Is editor.getSite().getWorkbenchWindow().getWorkbench().getDisplay(); preferable to editor.getSite().getShell().getDisplay(); ?
I'll make some initial changes to fix this, using widget.getDisplay() and editor.getSite().getWorkbenchWindow().getWorkbench().getDisplay(); In HistogramPane, using viewer.getControl().getDisplay() seems slightly risky as the viewer control is disposed and recreated, which might mean calling getDisplay on a disposed control, so using top seems safer.
editor.getSite().getWorkbenchWindow().getWorkbench().getDisplay(); fails on a non-UI thread with java.lang.IllegalStateException: No context available outside of the request service lifecycle. at org.eclipse.rwt.internal.service.ContextProvider.getContext(ContextProvider.java:107) at org.eclipse.rwt.internal.service.ContextProvider.getStateInfo(ContextProvider.java:160) at org.eclipse.rwt.SessionSingletonBase.getInstance(SessionSingletonBase.java:84) at org.eclipse.ui.internal.Workbench.getInstance(Workbench.java:378) at org.eclipse.ui.PlatformUI.getWorkbench(PlatformUI.java:94) at org.eclipse.ui.internal.WorkbenchWindow.getWorkbench(WorkbenchWindow.java:1584) at org.eclipse.mat.ui.QueryExecution.displayResult(QueryExecution.java:134) at org.eclipse.mat.ui.QueryExecution$ExecutionJob.run(QueryExecution.java:178) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) I'll revert to editor.getSite().getShell().getDisplay(); but should it work? Should getWorkbenchWindow cache its workbench?
New Gerrit change created: https://git.eclipse.org/r/c/mat/org.eclipse.mat/+/199628
Gerrit change https://git.eclipse.org/r/c/mat/org.eclipse.mat/+/199628 was merged to [master]. Commit: http://git.eclipse.org/c/mat/org.eclipse.mat.git/commit/?id=feaa9401d38f5540c5c404aac2aa3d1e9d8845e9
Should now be all done.
