Bugzilla – Full Text Bug Listing
| Summary: | [Progress] Animation error refreshing branches using RC1 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Olivier Thomann <Olivier_Thomann> | ||||
| Component: | CVS | Assignee: | Michael Valenta <Michael.Valenta> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P2 | CC: | gunnar, jean-michel_lemieux, john.arthorne, lphillips, michaelvanmeekeren, Tod_Creasey | ||||
| Version: | 3.0 | ||||||
| Target Milestone: | 3.0 RC2 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | 65950, 65951 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Olivier Thomann
moving to UI.
AnimationManager has a null progress monitor and isn't checking. I'm not sure
how this is spec'd but you should maybe protect against this case. BTW, the
steps will always dupe the problem.
public IStatus runInUIThread(IProgressMonitor monitor) {
if(monitor.isCanceled()) <===
return Status.CANCEL_STATUS;
animationProcessor.animationStarted();
Should address for RC2 The problem we are having is that it is possible for the monitor sent to a job to be null. I have checked progressFor() in ProgressManager and I could not have sent that without a ClassCastException. Note that the worker count will hit 300 easily with this operation and that the progress region gets updated constantly as a result. Moving to Core as this is a problem the progressMonitor caching. Did it not occur to anyone that 300 simultaneous worker threads was a sign of
bad things happening?
Here is what happens:
The refresh wizard has a loop to refresh each project in the repository:
for (int i = 0; i < selectedFolders.length; i++) {
ICVSRemoteResource resource = selectedFolders[i];
if (resource instanceof ICVSFolder) {
manager.refreshDefinedTags((ICVSFolder)resource, true /* replace */, true,
Policy.subMonitorFor(monitor, 100));
}
}
After each project is refreshed, a syncExec is fired to refresh the Repositories
view. Each node in the repository view does the fetching of its children using
IProgressService.run(true, true, runnable).
Each invocation of IProgressService.run causes a UI job to be scheduled that is
responsible for opening a progress dialog
(ProgressManager.scheduleProgressMonitorJob). This job looks like this:
public IStatus runInUIThread(IProgressMonitor monitor) {
setUserInterfaceActive(true);
if (ProgressManagerUtil.rescheduleIfModalShellOpen(this))
return Status.CANCEL_STATUS;
dialog.open();
return Status.OK_STATUS;
}
The interesting line is the one that reschedules the job if there is a modal
shell open. Of course, there *is* a modal shell open in this example: the
refresh branches dialog.
When I tried this, there were over 1,600 of these "open progress dialog" jobs
scheduled (number of projects times number of children to refresh in repo view).
Every one of them is repeatedly rescheduling itself in a loop for the duration
of the refresh operation. I did see the NPE mentioned above, but I also saw
dozens of other errors in the progress view code (I will attach a log). I
suspect this is causing a complete overload that is revealing timing bugs all
over the place.
Some possible areas of improvement:
- The CVS refresh operation could just fire a single syncExec at the end of the
refresh to update the repositories view, rather than one refresh per project.
- IProgressService.run should use a singleton job to open the progress dialog,
to avoid thousands of jobs from being stuck in this rescheduling loop.
I will leave this bug in core until the NPE is understood, but this is a serious
bug in the progress manager code as well.
Created attachment 11622 [details]
Log file showing various errors (out of handles, NPE, etc)
*** Bug 66553 has been marked as a duplicate of this bug. *** The NullPointerException no longer occurs in build I20040609. I suspect the massive flooding of jobs described in comment #4 was causing all kinds of problems (see my attached log), of which the NPE was just one. The ProgressManager code has been improved to no longer repeatedly reschedule the jobs responsible for opening the progress dialog. However, I will move this bug to CVS because this test case still causes far more work than necessary. Steps: 1) Create an anonymous connection to dev.eclipse.org 2) Expand HEAD 3) Refresh Branches 4) Select All 5) Finish You can see the repository view in the background flashing madly as the operation progresses. This should be fixed to refresh the tree only at the end of the operation. Since the operation runs in a modal dialog, the user can't browse any of the results until the entire refresh completes anyway, so these refreshes are all wasted. Fixed *** Bug 66621 has been marked as a duplicate of this bug. *** *** Bug 68185 has been marked as a duplicate of this bug. *** |