Bug 506103

Summary:	Provide API to attach a job to any progress monitor
Product:	[Eclipse Project] Platform	Reporter:	Mickael Istria <mistria>
Component:	UI	Assignee:	Platform-UI-Inbox <Platform-UI-Inbox>
Status:	NEW ---	QA Contact:
Severity:	enhancement
Priority:	P3	CC:	eclipse.sprigogin
Version:	4.6
Target Milestone:	---
Hardware:	All
OS:	All
Whiteboard:

Description Mickael Istria

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

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

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

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.