Bug 506103 - Provide API to attach a job to any progress monitor
Summary: Provide API to attach a job to any progress monitor
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-17 13:34 EDT by Mickael Istria CLA
Modified: 2016-10-17 15:02 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2016-10-17 13:34:17 EDT
In order to more easily provide progress report in wizards or other views, it would be great to allow to attach an existing progress monitor to a job, and get report on this progressMonitor as well as other ones (ProgressView).

Currently, eith https://git.eclipse.org/r/80925 , I've used ProgressManager.getInstance().progressFor(refreshProposalJob).addProgressMonitor(wizardMonitor) . We should consider either making this API public, or making a better API such as Job.attachProgressMonitor(monitor) and Job.detachProgressMonitor(monitor).
Comment 1 Sergey Prigogin CLA 2016-10-17 13:58:18 EDT
I'm a bit wary of exposing Job.attachProgressMonitor(monitor) and Job.detachProgressMonitor(monitor) as a public API since most progress monitor implementations are not thread safe. If we do expose these methods, they should carry strong warnings in their Javadoc.
Comment 2 Mickael Istria CLA 2016-10-17 14:38:02 EDT
(In reply to Sergey Prigogin from comment #1)
> I'm a bit wary of exposing Job.attachProgressMonitor(monitor) and
> Job.detachProgressMonitor(monitor) as a public API since most progress
> monitor implementations are not thread safe. If we do expose these methods,
> they should carry strong warnings in their Javadoc.

Makes sense to be wary, and to consider adding warnings.
However, the feature is IMO worth it, and since it's a new API that's entirely opt-out unless people use it directly in their code, we can assume that issues will be spotted at dev-time/test-time, that it will make people rework and improve their progress monitors to be thread-safe; and that there should no be bad surprises and broken things.
Comment 3 Sergey Prigogin CLA 2016-10-17 15:02:51 EDT
(In reply to Mickael Istria from comment #2)
> However, the feature is IMO worth it, and since it's a new API that's
> entirely opt-out unless people use it directly in their code, we can assume
> that issues will be spotted at dev-time/test-time, that it will make people
> rework and improve their progress monitors to be thread-safe; and that there
> should no be bad surprises and broken things.

There is a good reason why SubMonitor (the most commonly use implementation of IProgressMonitor) is not thread safe and should remain that way. That reason is speed. Any thread-safe implementation would be significantly slower than a carefully crafted one intended for a single thread use.