Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 324640

Summary: Improper use of IProgressMonitor in SystemTempFileListener (and likely elsewhere)
Product: [Tools] Target Management Reporter: Martin Oberhuber <mober.at+eclipse>
Component: RSEAssignee: dsdp.tm.rse-inbox <tm.rse-inbox>
Status: NEW --- QA Contact: Martin Oberhuber <mober.at+eclipse>
Severity: minor    
Priority: P3    
Version: 3.2   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:

Description Martin Oberhuber CLA 2010-09-07 06:51:24 EDT
Found when reviewing bug :

When SystemTempFileListener#synchronizeTempWithRemote() is called, its progress monitor is already initialized with

  monitor.beginTask(MSG_SYNCHRONIZE_PROGRESS, IProgressMonitor.UNKNOWN);

in various places down the call chain, that same progress monitor is passed unmodified into client calls which themselves may do a monitor.beginTask(). But monitor.beginTask() is documented as "This must only be called once on a given progress monitor instance".

For instance, fs.getRemoteFileObject() calls checkIsConnected() which may end up in a connect sequence that performs beginTask().

To clean this up, it looks like every call to client code should potentially open a SubProgressMonitor for the client code to have full control over. In other places at Eclipse, I have seen this done like

  Policy.subMonitorFor(monitor, workItems)

where the static method subMonitorFor() would return null in case monitor is null, or a SubProgressMonitor for the given number of work items in case a monitor was given.

I do not think that this is a severe problem in the concrete case, but thought I'd bring it up and document it.
Comment 1 Martin Oberhuber CLA 2010-09-07 06:52:33 EDT
Found when reviewing the change for bug 191284.