Community
Participate
Working Groups
Build id: 20090619-0625 I'm using P2Util from the prestartupdate example. If you pass a monitor into the engine.perform method, ie: IStatus status = engine.perform(profile, new DefaultPhaseSet(), plan.getOperands(), pc, sub.newChild(100, SubMonitor.SUPPRESS_ALL_LABELS)); Then later the worker threads responsible for downloading the files will attempt to update the progress bar. This causes an invalid thread access because these workers aren't in the UI thread. An easy workaround is simply not to pass a monitor into the engine.perform method. Stack trace below. Thread [Worker-0] (Suspended (exception SWTException)) SWT.error(int, Throwable, String) line: 3886 SWT.error(int, Throwable) line: 3799 SWT.error(int) line: 3770 ProgressBar(Widget).error(int) line: 696 ProgressBar(Widget).checkWidget() line: 341 ProgressBar.getSelection() line: 153 ProgressIndicator.worked(double) line: 139 ProgressMonitorDialog$ProgressMonitor.internalWorked(double) line: 246 ProgressMonitorDialog$ProgressMonitor.worked(int) line: 241 SubMonitor$RootInfo.worked(int) line: 284 SubMonitor.internalWorked(double) line: 570 SubMonitor.worked(int) line: 585 FileReader.handleTransferEvent(IFileTransferEvent) line: 161 UrlConnectionRetrieveFileTransfer(AbstractRetrieveFileTransfer).fireTransferReceiveDataEvent() line: 318 UrlConnectionRetrieveFileTransfer(AbstractRetrieveFileTransfer).handleReceivedData(byte[], int, double, IProgressMonitor) line: 213 AbstractRetrieveFileTransfer$1.performFileTransfer(IProgressMonitor) line: 141 FileReader(FileTransferJob).run(IProgressMonitor) line: 73 Worker.run() line: 55
I'm guessing you are calling ProgressMonitorDialog#run with fork=false, which assumes the operation is running entirely in the calling thread. I'm not sure what the right answer is here - you should generally be using fork=true if possible, which would avoid this problem. However, we're doing something a bit unusual here in p2, by reporting progress in a different thread from the one in which we were called. The IProgressMonitor contract doesn't disallow this, but maybe it should.
(In reply to comment #1) > However, we're doing something a bit unusual here in p2, by reporting progress > in a different thread from the one in which we were called. The > IProgressMonitor contract doesn't disallow this, but maybe it should. > See also a similar issue in bug 244246. This bug was a bit different because there was code getting a UI-based progress monitor and passing it to a job. However it's a similar issue. Who is responsible for making sure a progress monitor is safe to call outside of the UI thread? At the very least, p2 should document if the caller is supposed to ensure this. Marking this as an [engine] bug for this particular case, but I think it's relevant to any API in p2 that accepts a progress monitor.
I can't think how this wouldn't be the callers responsibility unless otherwise documented. Is there a non UI-thread progress monitor that we can use to synch back progress results to a UI thread?
Yes, the AccumulatingProgressMonitor in ModalContext gathers and dispatches progress updates to the UI thread. This is what is used in most cases, except here I am assuming they are using fork=false that would prevent the ModalContext gathering monitor to be used.
Did anybody find a workaround to use P2Util and leave the progress monitoring intact despite this bug? I just extracted the RCP auto update code into a separate plug-in so I can use it out of the box: http://github.com/ralfebert/org.eclipse.equinox.p2.autoupdate As part of this, I had to disable the progress monitoring, because it broke the file transfers: http://github.com/ralfebert/org.eclipse.equinox.p2.autoupdate/commit/add1547f03480fc1dd0e2f5fa3394345be927c7d Any ideas about this?
AccumulatingProgressMonitor would be the right thing to use here, but unfortunately it's only package visible in JFace. You could make a copy of it, or you could provide a (non-accumulating) simpler wrapper that dispatches everything through an async exec. Something like... IProgressMonitor uiSafeMonitor = new ProgressMonitorWrapper(monitor) { // every method gets wrapped in an async, like this... public void worked(int work) { display.asyncExec(new Runnable() { public void run() { super.worked(work); } }; } }; then this monitor is handed to the engine. For that matter, it should probably be used for the entire runnable with progress. I'll update the example to do this (in M4), but this should get you going.
Thanks for the workaround, I'll try this. For the moment the non-progressing progress monitor works fine as well :) btw: Maybe it would make sense to distribute a "org.eclipse.equinox.p2.autoupdate" bundle that contains the P2Util class and the Activator code as official plug-in? In my opinion this would be very helpful for RCP developers as headless update is very often required for RCP applications. btw: I just did a little tutorial about "p2 updates for Eclipse RCP applications" for my upcoming updated RCP training course materials: http://www.ralfebert.de/blog/eclipsercp/p2_updates_tutorial/
Hmmmm....I just reexamined this code as part of updating P2Util for bug 292907, and I don't really understand why the progress dialog is failing. It is using a modal context with fork=true under the covers. So something stranger is going on here than I first thought. Will need to rebuild the example and investigate further. > btw: Maybe it would make sense to distribute a > "org.eclipse.equinox.p2.autoupdate" bundle that contains the P2Util class and > the Activator code as official plug-in? In my opinion this would be very > helpful for RCP developers as headless update is very often required for RCP > applications. We are looking at providing a headless p2 operations API (see bug 292907 and bug 292996) and this has reduced P2Util to about 10 lines of code. The RCP app really needs to be the one deciding when to check (ie, prestartup in the workbench advisor) and what kind of progress reporting is desired. So I don't see the need to support P2Util now that we've encapsulated all the low level code into a simple UpdateOperation.
For 3.5, the patched P2Util class with disabled progress monitoring works fine so far. Do you have an example of the 10 lines of P2Util code required to do this using the new API? I'd be willing to give it a try to make sure it suffices the RCP use case... Still, it would be worth considering a one-line solution (like a flag in WorkbenchAdvisor) to activate P2 updates for RCP applications imo.
Take a look at P2Util (in the same prestartupdate example) in the branch called R3_6_api_cleanup. Note that it runs against the p2 bundles from the branch. We're hoping to merge back into HEAD early in M5.
(In reply to comment #9) > For 3.5, the patched P2Util class with disabled progress monitoring works fine > so far. Do you have an example of the 10 lines of P2Util code required to do > this using the new API? I'd be willing to give it a try to make sure it > suffices the RCP use case... Still, it would be worth considering a one-line > solution (like a flag in WorkbenchAdvisor) to activate P2 updates for RCP > applications imo. Ralf, the new API (and corresponding example changes) has been merged into HEAD and is available now in Eclipse 3.6 M5.
(In reply to comment #8) > Hmmmm....I just reexamined this code as part of updating P2Util for bug 292907, > and I don't really understand why the progress dialog is failing. It is using > a modal context with fork=true under the covers. So something stranger is > going on here than I first thought. Will need to rebuild the example and > investigate further. Ian and I looked at this a little bit last night. The problem is that the update check is happening at the wrong time in the workbench/workbench window startup sequence. The event loop has not been started yet, so ModalContext.run(...) does not honor the fork=true, and therefore does not provide a UI-safe progress monitor. The solution is to check for updates after the event loop is running. I need to examine the startup sequence closer to be sure, but something like WorkbenchWindowAdvisor.postWindowOpen
not sure which milestone yet, bugs first, then examples...
fixed in HEAD >20100526. Moved the update check to postWindowOpen() on the workbench window advisor. I also updated the product file to start ds so that the services could be found. Without this, the operation was not finding a profile registry. I also added a preference check so that when the app is restarting after an update, it won't check for updates again.
verified in Build id: I20100601-0800 via source inspection. However I noticed that we aren't tagging the examples since they aren't in the map file. Opened bug 315519 for this.
*** Bug 286839 has been marked as a duplicate of this bug. ***
