Bugzilla – Full Text Bug Listing
| Summary: | SubMonitor.newChild passes zero ticks to child | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Susan McCourt <susan> | ||||||||
| Component: | Components | Assignee: | John Arthorne <john.arthorne> | ||||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | john.arthorne, ob1.eclipse, pascal, sxenos, tjwatson | ||||||||
| Version: | 3.5 | ||||||||||
| Target Milestone: | 3.5 M5 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Susan McCourt
quick glance at the progress monitor code - everything seems to be handled properly all the way down, although the UI code that sets up the monitor uses N for number of monitor ticks where N is the number of repos being loaded. Possible rounding error? Many clients use N*100, etc. Will debug after test pass. marking M3, with only two repos in the repo list and nothing ever loaded (which is the state of a fresh SDK), it can make you think you are hung while the ganymede content.jar downloads. Changing the target milestone as we are not contributing to M3. However please make sure these bugs are addressed early in M4 or tomorrow Testing Thursday for the test related ones. For some reason, I expected that a parent monitor's ticks would be updated appropriately if a submonitor were being updated. I wrote a snippet to play with this and now realize that the updating of the parent is atomic. The submonitor's progress won't affect the parent monitor's progress until the submonitor is done. From a p2 point of view, it means that our progress can sit at 0 for a long time while the first repo is loading, even though the details of the download can be seen progressing in the details view or progress view. If there are lots of repos, you'll see progress updating after each repo loads. But if you only have one repo, you sit at 0 until the very end when you update to 100%. The only thing I can think of to do here is to indicate a little bit of progress before the download starts. The UI code could do this, or even IMetadataRepository.loadRepository(URI, IProgressMonitor) could work the monitor before passing the rest of it down. Another idea might be for the UI to special case when only one repo will be loaded, and rather than creating a submonitor, it could pass the monitor directly. At least then the load of one repo could proceed in the top level progress. Not sure if this helps or not, as it depends on the pattern used all the way down the call chain. I mentioned this to Susan offline, but this isn't the expected behaviour of SubMonitor. It should be sending units of work to the parent monitor as work is being done. Created attachment 119846 [details]
sample command handler that uses submonitors
Ok, so it *should* be working the way I had assumed.
John said he could take a look, but I can open a platform bug if this is anything other than me making a dumb mistake.
Attached is a simple command handler that starts a job and runs a submonitor. I had hoped to see the parent monitor tick for every unit of work. It only updates when the child monitor completes.
I tried to debug this and I don't see how using SubMonitor.newChild would ever cause the right thing to happen, because it creates the new child using 0 for "availableToChildren" and that in turn causes no work to ever be consumed in this child.
This looks like a bug in SubMonitor to me. SubMonitor.newChild always instantiates the child monitor with 0 units of work available. Created attachment 119985 [details]
Potential fix
With this patch the progress in Susan's example runs smoothly. I haven't run further tests on it yet.
Assigning to John to release for M5. Oleg, I'd appreciate a second look from you since you've worked with SubMonitor in the past. It really surprised me that there would be a bug like this and I want to make sure I'm not missing something. Created attachment 120084 [details]
Patch with a JUnit test
Yes, this seems like a bug. John's fix look good.
I added JUnit and updated copyrights.
Fix and test released. Reopening. This was not a bug in SubMonitor -- it was a bug in whatever code invoked newChild but failed to invoke setWorkRemaining or beginTask to allocate ticks. The fix is making it harder to detect this class of bugs using the new progress error detection. Changing resolution to WONTFIX. I plan to revert the fix with the patch to bug 482062. |