Comment 1 Krzysztof Daniel 2008-08-20 08:31:58 EDT

Created attachment 110442 [details]
Patch suggestion

This seems to be somewhat complicated.
I think that the real problem here is that StatusLineManager creates some Composite and casts it to the IProgressManager (see getProgressMonitorDelegate), moreover, StatusLineManager is subclassable, so one can override createControl methods and get Composite that does not implement IProgressManager, which will cause ClassCastException.
In my opinion it is necessary to modify StatusLineManager#createControl javadoc, and then, as a consequence, add UI thread safety to StatusLine (which is internal, anyway).

To be honest I am not happy with my patch and any suggestion is welcome.

Comment 2 Dan Heidinga 2008-09-17 12:03:23 EDT

Created attachment 112781 [details]
Catches case missed by previous patch

I started running with the previous patch and was still getting some illegal thread access problems.  I've updated the patch to catch those cases and fixed an NPE while I was at it.

Comment 3 Susan McCourt 2008-09-17 13:29:54 EDT

I'll look this over for M3.

Comment 4 Dan Heidinga 2008-09-17 13:58:17 EDT

Any chance of getting this into 3.4.1?

Comment 5 Boris Bokowski 2008-09-17 14:25:14 EDT

(In reply to comment #4)
> Any chance of getting this into 3.4.1?

No - we are well past the point where we could have considered this for 3.4.1.

As a workaround, you can pass a wrapped progress monitor that forwards the setCanceled() using an async exec.

Comment 6 Dan Heidinga 2008-09-17 14:38:01 EDT

In reply to comment 5:

This occurs on almost all calls through the IProgressMonitor api.  It is not limited to just #setCanceled().

See also:
  public void beginTask(String name, final int totalWork);
  public void done();
  public void internalWorked(final double work);
  protected void updateMessageLabel();

plus the NPE in public void subTask(String name).

Having to wrap the progress monitor is a pretty bogus solution when there is a real fix in hand.

Comment 7 Dan Heidinga 2008-09-17 16:07:39 EDT

Created attachment 112818 [details]
Ensure #setCancel actually passes the canceled flag all the way down

New patch to also catch the case where the window that provided the StatusLine has been closed while the IProgressMonitor still lives.  In this case, it was impossible to set the monitor canceled and any job using the monitor became uncancelable.

Comment 8 Boris Bokowski 2008-09-17 22:38:13 EDT

(In reply to comment #6)
> Having to wrap the progress monitor is a pretty bogus solution when there is a
> real fix in hand.

I agree, but it's the only idea I have for you in the current situation, given that we cannot do anything about this in 3.4.1.

Comment 9 Dan Heidinga 2008-09-18 08:13:11 EDT

In reply to comment 8:

I agree.  I had the wrong dates in mind for the 3.4.1 release and had thought there was still time to get it in.  Sorry.

Comment 10 Susan McCourt 2008-10-23 16:00:14 EDT

Dan, I'm reviewing the patch, but want to understand better what it is you are trying to do.

> Steps To Reproduce:
> 1.  Pass the StatusLine's IProgressMonitor to a non-UI job
> 2.  Call setCanceled(true)
> 3.  See the SWTException: Invalid thread access

Are you directly invoking the run(IProgressMonitor) method on a non-UI job and passing it the status line's monitor?  Or did you set your own ProgressProvider into the JobManager?

The usual pattern is for the client to schedule the job, and the job manager uses the ProgressProvider to select a progress monitor for the job.  The ProgressProvider would be in charge of making sure, for example, that a UI object wasn't directly handed to the a non-UI job.  The workbench ProgressProvider (ProgressManager) ensures that all of the UI work when updating progress for jobs is performed in the UI thread.  (It creates a WorkbenchJob specifically for this purpose).

The spec for Job states that clients should not ever call the run method directly, but should instead schedule the job. 

So I'm not sure that the wrapped progress monitor suggestion is inappropriate for a progress provider that wishes to use the status line monitor for non-UI jobs. Or if you are invoking the run method directly and handing a monitor to your job, then you are working somewhat outside the spec of the Job API and again, a wrapper seems reasonable.

In general we assume that UI API is called from the UI thread.  I agree it's somewhat bogus that a UI object is disguised as an IProgressMonitor, but the code that is getting the monitor from the StatusLineManager and passing it along to a job or otherwise invoking it in a non-UI thread seems to be the one in violation of spec.

I'm not sure it's appropriate to add asyncs to the progress monitor.

Comment 11 Dan Heidinga 2008-10-23 18:03:45 EDT

(In reply to comment #10)
> Dan, I'm reviewing the patch, but want to understand better what it is you are
> trying to do.
> 
> > Steps To Reproduce:
> > 1.  Pass the StatusLine's IProgressMonitor to a non-UI job
> > 2.  Call setCanceled(true)
> > 3.  See the SWTException: Invalid thread access
> 
> Are you directly invoking the run(IProgressMonitor) method on a non-UI job and
> passing it the status line's monitor?  Or did you set your own ProgressProvider
> into the JobManager?

I'm not doing either.  I have a subclass of ApplicationWindow that needs to be notified of progress in its StatusLine.  I pass the StatusLineManager's IProgressMonitor to the constructor of a Job that represents something happening outside of Eclipse (in my case, in a Smalltalk image).  This is in turn passed to my progress daemon, code that listens for progress events from the image, and updates the progress monitor appropriately.

> 
> The usual pattern is for the client to schedule the job, and the job manager
> uses the ProgressProvider to select a progress monitor for the job.  The
> ProgressProvider would be in charge of making sure, for example, that a UI
> object wasn't directly handed to the a non-UI job.  The workbench
> ProgressProvider (ProgressManager) ensures that all of the UI work when
> updating progress for jobs is performed in the UI thread.  (It creates a
> WorkbenchJob specifically for this purpose).
> 
> The spec for Job states that clients should not ever call the run method
> directly, but should instead schedule the job.

I don't directly call the the #run() method.  Regardless of what the "normal" pattern is, code that is meant to decouple the UI and non-UI aspects needs to be safe to run in non-UI threads.
 
> 
> So I'm not sure that the wrapped progress monitor suggestion is inappropriate
> for a progress provider that wishes to use the status line monitor for non-UI
> jobs. Or if you are invoking the run method directly and handing a monitor to
> your job, then you are working somewhat outside the spec of the Job API and
> again, a wrapper seems reasonable.

If the StatusLine's progress monitor doesn't want to be callable from non-UI code, then it simply shouldn't be an IProgressMonitor.  Invent something else, an IUIProgressMonitor which has the the same API but can only be called in the UI thread.

> 
> In general we assume that UI API is called from the UI thread. I agree it's
> somewhat bogus that a UI object is disguised as an IProgressMonitor, but the
> code that is getting the monitor from the StatusLineManager and passing it
> along to a job or otherwise invoking it in a non-UI thread seems to be the one
> in violation of spec.

I agree UI code needs to be invoked in the UI thread.  Hiding UI code behind an IProgressMonitor, which exists to decouple UI and non-UI portions of the code, is completely bogus.  If you provide an IProgressMonitor, it needs to meet the requirements of an IProgressMonitor - namely that its safe to call from outside the UI thread.

> 
> I'm not sure it's appropriate to add asyncs to the progress monitor.
> 

Then please call it something other than an IProgressMonitor.  If the API contract is not going to be followed, don't confuse 

Comment 12 Boris Bokowski 2008-10-23 23:16:20 EDT

Fixed by adding the following clarification to the Javadoc of IStatusLineManager.getProgressMonitor:

 * Returns a progress monitor which reports progress in the status line.
 * Note that the returned progress monitor may only be accessed from the UI
 * thread.

(In reply to comment #11)
> If you provide an IProgressMonitor, it needs to meet the requirements
> of an IProgressMonitor - namely that its safe to call from outside
> the UI thread.

What you describe may make sense from your point of view, but I have not found this "requirement" in our Javadoc. (Or have I missed something?)

Comment 13 Dan Heidinga 2008-10-27 09:35:18 EDT

(In reply to comment #12)
> Fixed by adding the following clarification to the Javadoc of
> IStatusLineManager.getProgressMonitor:
> 
>  * Returns a progress monitor which reports progress in the status line.
>  * Note that the returned progress monitor may only be accessed from the UI
>  * thread.
> 

Can you please explain to me why changing the StatusLine's ProgressMonitor to be more robust and callable from outside the UI thread is a bad thing?  I don't see this javadoc change as a fix for the SWTException.  I'm also confused by the resistance to apply the patch.  If there is some reason why this is a bad patch, or causes problems, please include that information in this bug.

> (In reply to comment #11)
> > If you provide an IProgressMonitor, it needs to meet the requirements
> > of an IProgressMonitor - namely that its safe to call from outside
> > the UI thread.
> 
> What you describe may make sense from your point of view, but I have not found
> this "requirement" in our Javadoc. (Or have I missed something?)
> 

The "requirement" is called out in "The Official Eclipse FAQs" which states that
    "Thus, an IProgressMonitor is an abstraction that allows for decoupling of UI and non-UI components."
(http://wiki.eclipse.org/FAQ_How_do_I_use_progress_monitors%3F)

Granted, this isn't the javadoc, but it is from eclipse.org.  Also, considering IProgressMonitor isn't defined in a UI-specific package, and the way it is used throughout the platform's concurrency support, it seems unnecessarily difficult to determine when an IProgressMonitor is safe to use outside the UI. 

IProgressMonitors do not provide anything that would let a user know they need to wrap all progress calls in async or sync execs (ie: no IProgressMonitor#isUISafe() method).  By pushing the requirement down to the user, it seems to unnecessarily complicate things everywhere progress monitors are used, as only the creator of the IProgressMonitor has the information to know if, and where, the monitor actually touches the UI.

Comment 14 Susan McCourt 2008-10-27 15:33:17 EDT

(In reply to comment #13)
<stuff deleted>
> Can you please explain to me why changing the StatusLine's ProgressMonitor to
> be more robust and callable from outside the UI thread is a bad thing?  I don't
> see this javadoc change as a fix for the SWTException.  I'm also confused by
> the resistance to apply the patch.  If there is some reason why this is a bad
> patch, or causes problems, please include that information in this bug.

I think it's a matter of weighing the work to support thread safety in a UI object vs. the workaround that is available.  It's a long standing practice that UI objects are assumed to be called from the UI thread and the client is not guaranteed anything with regards to thread safety or access from a non-UI thread unless it is explicitly documented as such.  

There are all kinds of "non-UI" classes that can be obtained through API from a UI object.  Model objects, collections, etc.  There is no general guarantee that a non-UI object obtained from a UI object should be callable from any thread.

Using async blocks in the progress monitor API works with your usage pattern, but we'd have to consider other patterns, whether any other synchronization is required, etc.  There is also the problem of inconsistency.  Why do we support this particular case?  Do we then apply the same standard to ProgressMonitorDialog.getProgressMonitor() and any other UI components that have API for progress monitors?  What about other non-UI objects returned by UI components?

I'm not trying to be difficult or dogmatic here, just explaining why we can't just apply a patch that fixes one usage pattern without considering the integrity/consistency of the component. 

<snip>

> The "requirement" is called out in "The Official Eclipse FAQs" which states
> that
>     "Thus, an IProgressMonitor is an abstraction that allows for decoupling of
> UI and non-UI components."
> (http://wiki.eclipse.org/FAQ_How_do_I_use_progress_monitors%3F)

I don't conside decoupling of UI from core at the API level to be the same as non-UI thread safety.  I agree it's open to interpretation.

> it seems unnecessarily difficult
> to determine when an IProgressMonitor is safe to use outside the UI. 

A good rule of thumb is don't pass a progress monitor from a UI component to a non-UI thread without guarding it with async blocks.
> 
> IProgressMonitors do not provide anything that would let a user know they need
> to wrap all progress calls in async or sync execs (ie: no
> IProgressMonitor#isUISafe() method).  By pushing the requirement down to the
> user, it seems to unnecessarily complicate things everywhere progress monitors
> are used, as only the creator of the IProgressMonitor has the information to
> know if, and where, the monitor actually touches the UI.
> 

If the problem here was that code *using* the progress monitor was expected to handle UI threading issues, or even call a method like monitor.isThreadSafe(), I would completely agree with your arguments. No way should some random task have to check the "UI-ness" of its progress monitor.  That is what the decoupling is all about.

However, it seems reasonable to me that the code that creates/gets a monitor from some UI object and passes it along to a non-UI thread has some responsibility here. 

Marking FIXED because I believe the issue is different interpretations of the API contract and the javadoc correction makes it more explicit.

Comment 15 Susan McCourt 2008-10-27 15:46:55 EDT

.

Comment 16 Dan Heidinga 2008-10-27 16:06:55 EDT

(In reply to comment #14)
> (In reply to comment #13)
> <stuff deleted>
> > Can you please explain to me why changing the StatusLine's ProgressMonitor to
> > be more robust and callable from outside the UI thread is a bad thing?  I don't
> > see this javadoc change as a fix for the SWTException.  I'm also confused by
> > the resistance to apply the patch.  If there is some reason why this is a bad
> > patch, or causes problems, please include that information in this bug.
> 
> I think it's a matter of weighing the work to support thread safety in a UI
> object vs. the workaround that is available.  It's a long standing practice
> that UI objects are assumed to be called from the UI thread and the client is
> not guaranteed anything with regards to thread safety or access from a non-UI
> thread unless it is explicitly documented as such.
> 
> There are all kinds of "non-UI" classes that can be obtained through API from a
> UI object.  Model objects, collections, etc.  There is no general guarantee
> that a non-UI object obtained from a UI object should be callable from any
> thread.


It seems the disagreement here is whether or not an IProgressMonitor is a UI object.  If the IProgressMonitor is a UI object, then I agree it needs to be used from the UI Thread.  My position though, is that the IProgressMonitor is NOT a UI object.  

The quote from the Eclipse FAQ states that they exist to decouple the UI and non-UI portions of the code.  In SWT's case, one of the ways code is coupled to the UI is through the requirement to run in the UI.  Basically, claiming the progress monitor is a UI object that requires sync/async execs by the user prevents this monitor from ever being used in a non-UI plugin.  This seems like a major handicap.


> 
> Using async blocks in the progress monitor API works with your usage pattern,
> but we'd have to consider other patterns, whether any other synchronization is
> required, etc.  There is also the problem of inconsistency.  Why do we support
> this particular case?  Do we then apply the same standard to
> ProgressMonitorDialog.getProgressMonitor() and any other UI components that
> have API for progress monitors?  What about other non-UI objects returned by UI
> components?

I would argue that the same sort of fix IS needed for ProgressMonitorDialog.getProgressMonitor().  This looks to me to be a case where the only code that "knows" if a given progress monitor implementation is UI safe, is the code that implements the monitor.  Therefore, it is the responsibility of the implementation to provide the necessary UI safety.

This is also a case where if the change is made here, then it only requires one change, rather than everyone trying to use it requiring their own custom wrapper.

> 
> I'm not trying to be difficult or dogmatic here, just explaining why we can't
> just apply a patch that fixes one usage pattern without considering the
> integrity/consistency of the component. 

I understand.  I'm not trying to cause trouble either, I just want the best possible platform to build with.
> 
> <snip>
> 
> > The "requirement" is called out in "The Official Eclipse FAQs" which states
> > that
> >     "Thus, an IProgressMonitor is an abstraction that allows for decoupling of
> > UI and non-UI components."
> > (http://wiki.eclipse.org/FAQ_How_do_I_use_progress_monitors%3F)
> 
> I don't conside decoupling of UI from core at the API level to be the same as
> non-UI thread safety.  I agree it's open to interpretation.

I don't see how they can be considered decoupled if the code is only usable from UI plugins.  There is no way (I know of) for a non-UI plugin to do an async/sync exec.  This seems then to tightly couple the IProgressMonitor to the UI.

> 
> > it seems unnecessarily difficult
> > to determine when an IProgressMonitor is safe to use outside the UI. 
> 
> A good rule of thumb is don't pass a progress monitor from a UI component to a
> non-UI thread without guarding it with async blocks.

A better solution would be for the UI to not create IProgressMonitors that are only usable from the UI.  If you don't want to make the changes to the StatusLine, then it seems like a great candidate for the platform to create a single UI safe progressMonitor wrapper and wrap all IProgressMonitors it creates with the wrapper.  

This is a fix in one place, rather than everyone using it requiring their own fixes.

> > 
> > IProgressMonitors do not provide anything that would let a user know they need
> > to wrap all progress calls in async or sync execs (ie: no
> > IProgressMonitor#isUISafe() method).  By pushing the requirement down to the
> > user, it seems to unnecessarily complicate things everywhere progress monitors
> > are used, as only the creator of the IProgressMonitor has the information to
> > know if, and where, the monitor actually touches the UI.
> > 
> 
> If the problem here was that code *using* the progress monitor was expected to
> handle UI threading issues, or even call a method like monitor.isThreadSafe(),
> I would completely agree with your arguments. No way should some random task
> have to check the "UI-ness" of its progress monitor.  That is what the
> decoupling is all about.
> 
> However, it seems reasonable to me that the code that creates/gets a monitor
> from some UI object and passes it along to a non-UI thread has some
> responsibility here.

This will just lead to having numerous different solutions all doing the same thing.  Lets fix it once, and ensure that no one ever has to deal with this problem again.
 
> 
> Marking FIXED because I believe the issue is different interpretations of the
> API contract and the javadoc correction makes it more explicit.
> 

I respsectfully disagree.

Comment 17 Susan McCourt 2008-10-27 17:29:23 EDT

Not to beat a dead horse, but just to clarify.

> It seems the disagreement here is whether or not an IProgressMonitor is a UI
> object.  If the IProgressMonitor is a UI object, then I agree it needs to be
> used from the UI Thread.  My position though, is that the IProgressMonitor is
> NOT a UI object.  

We agree on this one.  IProgressMonitor is NOT a UI object. 
But StatusLine is, and when a non-UI object is obtained from a UI object and passed outside of the UI thread, that's where the trouble starts. The StatusLine client must use judgment here, there's no guarantee that objects from the StatusLine can be accessed safely outside of the UI thread.  

>Basically, claiming the
> progress monitor is a UI object that requires sync/async execs by the user
> prevents this monitor from ever being used in a non-UI plugin.  This seems like
> a major handicap.

As I said in previous comment, I agree it would be ridiculous to expect all clients of IProgressMonitor to perform this check, and as you point out, impossible if the code runs headless.  But some code in a UI plug-in retrieved this monitor from the StatusLine.  That's where the wrapper needs to be, not in the underlying class library.

>This looks to me to be a case
> where the only code that "knows" if a given progress monitor implementation is
> UI safe, is the code that implements the monitor.  Therefore, it is the
> responsibility of the implementation to provide the necessary UI safety.

This is where I respectfully disagree.  SWT and JFace are to be used in the UI thread unless documented otherwise.  That decision was made on the fundamental premise that multi-threaded UI implementations should take on the burden and overhead of adding thread-safety, rather than build it into the UI classes and make all clients pay for it.  Thus the async/sync methods are provided to allow clients that knowingly work with multiple threads to call into the UI. 

>I understand.  I'm not trying to cause trouble either, I just want the best
>possible platform to build with.

Me, too.  That's why I want to be consistent in placing the burden on the platform UI client.

I agree this particular case is a gray area, but in these instances I typically choose consistency unless there's a compelling error or inability to work around it.  Not to mention you have to be in violation of the job spec somewhere to cause this, by calling the run method directly and circumventing the ProgressProvider.

I double checked the doc on SWT/Threading (http://help.eclipse.org/stable/index.jsp?topic=/org.eclipse.platform.doc.isv/guide/swt_threading.htm), and even though it lays out some rules of engagement, it still doesn't really help settle this one.

"a platform UI library should not be considered thread-safe unless it is specifically documented as such."

"In general, any workbench UI extensions you add to the platform will be executing in the workbench's UI thread, unless they are specifically related to threads or background jobs (such as background job progress indication)."

Comment 18 Dan Heidinga 2008-10-27 21:39:19 EDT

(In reply to comment #17)
> Not to beat a dead horse, but just to clarify.
> 
> > It seems the disagreement here is whether or not an IProgressMonitor is a UI
> > object.  If the IProgressMonitor is a UI object, then I agree it needs to be
> > used from the UI Thread.  My position though, is that the IProgressMonitor is
> > NOT a UI object.  
> 
> We agree on this one.  IProgressMonitor is NOT a UI object. 
> But StatusLine is, and when a non-UI object is obtained from a UI object and
> passed outside of the UI thread, that's where the trouble starts. The
> StatusLine client must use judgment here, there's no guarantee that objects
> from the StatusLine can be accessed safely outside of the UI thread.

I agree the StatusLine is a UI object.  The problem is that the progress monitor obtained by called StatusLineManager#getProgressMonitor is not safe to be accessed outside the UI.  If I was casting a StatusLine to a IProgressMonitor, the responsibility would be mine, but since the monitor is being returned by API, it is the responsibility of that API to return a monitor that is properly decoupled from the UI.

> 
> >Basically, claiming the
> > progress monitor is a UI object that requires sync/async execs by the user
> > prevents this monitor from ever being used in a non-UI plugin.  This seems like
> > a major handicap.
> 
> As I said in previous comment, I agree it would be ridiculous to expect all
> clients of IProgressMonitor to perform this check, and as you point out,
> impossible if the code runs headless.  But some code in a UI plug-in retrieved
> this monitor from the StatusLine.  That's where the wrapper needs to be, not in
> the underlying class library.

I agree the wrapper needs to be at the place where the plugin gets the IProgressMonitor - and that place IS the StatusLine.  If there was no #getProgressMonitor call on StatusLineManager, and converting it required a cast in user code, than you would be correct.  The fact that it returns a IProgressMonitor means it needs to be a valid IProgressMonitor. 

> 
> >This looks to me to be a case
> > where the only code that "knows" if a given progress monitor implementation is
> > UI safe, is the code that implements the monitor.  Therefore, it is the
> > responsibility of the implementation to provide the necessary UI safety.
> 
> This is where I respectfully disagree.  SWT and JFace are to be used in the UI
> thread unless documented otherwise.  That decision was made on the fundamental
> premise that multi-threaded UI implementations should take on the burden and
> overhead of adding thread-safety, rather than build it into the UI classes and
> make all clients pay for it.  Thus the async/sync methods are provided to allow
> clients that knowingly work with multiple threads to call into the UI.

IProgressMonitor is NOT an SWT or JFace interface.  Therefore, requiring async/sync execs to use it is invalid.  If I was accessing a UI object from outside the UI, the fault would rest with me.  As I am asking the UI for a non-UI object, I should be safe to use it outside of the UI.  
 
> 
> >I understand.  I'm not trying to cause trouble either, I just want the best
> >possible platform to build with.
> 
> Me, too.  That's why I want to be consistent in placing the burden on the
> platform UI client.
> 
> I agree this particular case is a gray area, but in these instances I typically
> choose consistency unless there's a compelling error or inability to work
> around it.  Not to mention you have to be in violation of the job spec
> somewhere to cause this, by calling the run method directly and circumventing
> the ProgressProvider.

This is the second time you have made this claim and I'm confused by it.  It is valid to ask an ApplicationWindow for its StatusLineManager and the StatusLineManager for its progress monitor, and to then use this progress monitor to indicate progress.  IProgressMonitors do not have to be used in conjunction with the Job framework.  It is just as valid to use them with a separate Java thread.  I do not directly call Job#run(IProgressMonitor) or circumvent the ProgressProvider and still have this problem.  Please provide an explanation or link to back up your claim.

> 
> I double checked the doc on SWT/Threading
> (http://help.eclipse.org/stable/index.jsp?topic=/org.eclipse.platform.doc.isv/guide/swt_threading.htm),
> and even though it lays out some rules of engagement, it still doesn't really
> help settle this one.
> 
> "a platform UI library should not be considered thread-safe unless it is
> specifically documented as such."
> 
> "In general, any workbench UI extensions you add to the platform will be
> executing in the workbench's UI thread, unless they are specifically related to
> threads or background jobs (such as background job progress indication)."
> 

I think your second link actually shows the StatusLine to be against the rules of engagement: 
   "(such as background job progress indication)"  - Seems to me that any implementation of IProgressMonitor can be used for background job progress indication....

Comment 19 Susan McCourt 2008-10-28 11:51:13 EDT

(In reply to comment #18)
> I agree the StatusLine is a UI object.  The problem is that the progress
> monitor obtained by called StatusLineManager#getProgressMonitor is not safe to
> be accessed outside the UI.  If I was casting a StatusLine to a
> IProgressMonitor, the responsibility would be mine, but since the monitor is
> being returned by API, it is the responsibility of that API to return a monitor
> that is properly decoupled from the UI.

That is your assertion, I understand it.  We've made the spec clearer that this is not true.  We can debate all day whether the wrapper should happen before the call or made by the client.  Consistency vs. case-by-case wrappering is my concern, as I've tried to communicate.

> This is the second time you have made this claim and I'm confused by it.  It is
> valid to ask an ApplicationWindow for its StatusLineManager and the
> StatusLineManager for its progress monitor, and to then use this progress
> monitor to indicate progress.  IProgressMonitors do not have to be used in
> conjunction with the Job framework.  It is just as valid to use them with a
> separate Java thread.  I do not directly call Job#run(IProgressMonitor) or
> circumvent the ProgressProvider and still have this problem.  Please provide an
> explanation or link to back up your claim.

You said you are directly linking the Job and the IProgressMonitor in comment #1, and comment #11 describes the job passing that monitor to the progress daemon.  If the job was not run directly, then the job is ignoring the progress monitor provided in its run() method and instead passing the status line progress monitor.  Otherwise the JobManager would scheudle the job using the ProgressProvider's progress.  The end result is the job is using a non-UI safe progress monitor.

I believe I understand all your arguments, and agree that this might be a reasonable improvement if applied throughout platform UI.  (Although I disagree with the use of syncExec in the patch and would instead use asyncs).

Pragmatics are that this has been the state of StatusLine's progress monitor API since it was first checked into CVS, there are much bigger fish to fry.  I'm reopening/retitling the bug so that you can vote for it if you wish, but honestly I see this as low prio unless we see more votes for it.

Comment 20 Susan McCourt 2009-07-09 18:19:44 EDT

As per http://wiki.eclipse.org/Platform_UI/Bug_Triage_Change_2009

Comment 21 Eclipse Webmaster 2019-09-06 16:03:37 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

Comment 22 Eclipse Genie 2021-09-15 12:12:22 EDT

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.