Skip to content

Commit

Permalink
Allow the ProgressProvider to control the used instance of a monitor
Browse files Browse the repository at this point in the history
This enables the ProgressProvider to decide the instance of the
ProgressMonitor used in beginRule / join / suspend operations.

This is important if the progressprovider needs to react to certain
actions, e.g. in an UI a provider might make sure the UI is responsive
or show a "terminate" button even if the job is currently waiting on a
rule.

Fix eclipse-platform#40
  • Loading branch information
laeubi committed Apr 28, 2022
1 parent c45ef62 commit 7ec08ed
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2003, 2020 IBM Corporation and others.
* Copyright (c) 2003, 2022 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -20,7 +20,8 @@
* Jan Koehnlein - Fix for bug 60964 (454698)
* Terry Parker - Bug 457504, Publish a job group's final status to IJobChangeListeners
* Xored Software Inc - Fix for bug 550738
* Christoph Laeubrich - remove deprecated API
* Christoph Laeubrich - remove deprecated API
* - Issue #40 - UI freezes when a ThreadJob is waiting to run in the UI Thread
*******************************************************************************/
package org.eclipse.core.internal.jobs;

Expand Down Expand Up @@ -1100,19 +1101,27 @@ boolean join(InternalJobGroup jobGroup, long timeout, IProgressMonitor monitor)
}

/**
* Returns a non-null progress monitor instance. If the monitor is null,
* returns the default monitor supplied by the progress provider, or a
* NullProgressMonitor if no default monitor is available.
* Returns a non-null progress monitor instance. If a progress provider is set,
* it is asked to provide a wrapped instance of the given <code>monitor</code>
* parameter. If no progress provider is set and <code>monitor</code> is null, a
* NullProgressMonitor is returned in all other cases the given
* <code>monitor</code> instance.
*
* @param monitor the monitor instance (could be null) for further usage
* @return a non-null progress monitor instance.
*/
private IProgressMonitor monitorFor(IProgressMonitor monitor) {
if (monitor == null || (monitor instanceof NullProgressMonitor)) {
if (progressProvider != null) {
try {
monitor = progressProvider.getDefaultMonitor();
} catch (Exception e) {
String msg = NLS.bind(JobMessages.meta_pluginProblems, JobManager.PI_JOBS);
RuntimeLog.log(new Status(IStatus.ERROR, JobManager.PI_JOBS, JobManager.PLUGIN_ERROR, msg, e));
if (progressProvider != null) {
try {
IProgressMonitor monitorFor = progressProvider.monitorFor(monitor);
if (monitorFor == null) {
throw new IllegalStateException("Internal error: " + //$NON-NLS-1$
progressProvider.getClass().getName() + "#monitorFor(" + monitor + ") returned null!"); //$NON-NLS-1$ //$NON-NLS-2$
}
return monitorFor;
} catch (Exception e) {
String msg = NLS.bind(JobMessages.meta_pluginProblems, JobManager.PI_JOBS);
RuntimeLog.log(new Status(IStatus.ERROR, JobManager.PI_JOBS, JobManager.PLUGIN_ERROR, msg, e));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2003, 2012 IBM Corporation and others.
* Copyright (c) 2003, 2022 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -10,6 +10,7 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
* Christoph Läubrich - Issue #40 - UI freezes when a ThreadJob is waiting to run in the UI Thread
*******************************************************************************/
package org.eclipse.core.runtime.jobs;

Expand Down Expand Up @@ -96,4 +97,28 @@ public IProgressMonitor createMonitor(Job job, IProgressMonitor group, int ticks
public IProgressMonitor getDefaultMonitor() {
return new NullProgressMonitor();
}

/**
* Returns a (possibly wrapped) progress monitor to use when running in a job.
* <p>
* The default implementation does the following:
* <ul>
* <li>if <code>monitor</code> is null or an instance of
* {@link NullProgressMonitor} returns the value from a call to
* ProgressProvider#getDefaultMonitor()</li>
* <li>in all other cases return the the <code>monitor</code> passed to this
* method.
* </ul>
*
* @param monitor the monitor to wrap, might be null
* @return a progress monitor for the given <code>monitor</code> but never
* <code>null</code>
* @since 3.13
*/
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
if (monitor == null || monitor instanceof NullProgressMonitor) {
return getDefaultMonitor();
}
return monitor;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
*******************************************************************************/
package org.eclipse.core.tests.runtime.jobs;

import java.util.concurrent.atomic.AtomicIntegerArray;
import java.util.concurrent.atomic.*;
import org.eclipse.core.runtime.*;
import org.eclipse.core.runtime.jobs.*;
import org.eclipse.core.tests.harness.FussyProgressMonitor;
import org.eclipse.core.tests.harness.TestBarrier2;
import org.junit.Test;

/**
* Tests API methods IJobManager.beginRule and IJobManager.endRule
Expand Down Expand Up @@ -50,6 +51,39 @@ public void run() {
}
}

@Test
public void testRuleCallsProgressProvider_monitorFor() {
AtomicBoolean createdMonitor = new AtomicBoolean();
AtomicReference<IProgressMonitor> passedMonitor = new AtomicReference<>();
manager.setProgressProvider(new ProgressProvider() {

@Override
public IProgressMonitor createMonitor(Job job) {
return new NullProgressMonitor();
}

@Override
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
assertEquals(passedMonitor.get(), monitor);
createdMonitor.set(true);
return super.monitorFor(monitor);
}
});
IdentityRule rule = new IdentityRule();
IProgressMonitor[] monitors = new IProgressMonitor[] { null, new NullProgressMonitor(),
SubMonitor.convert(null) };
for (IProgressMonitor monitor : monitors) {
createdMonitor.set(false);
passedMonitor.set(monitor);
manager.beginRule(rule, monitor);
try {
assertTrue("Monitor not created for " + monitor, createdMonitor.get());
} finally {
manager.endRule(rule);
}
}
}

public void testComplexRuleStarting() {
//test how the manager reacts when several different threads try to begin conflicting rules
final int NUM_THREADS = 3;
Expand Down

0 comments on commit 7ec08ed

Please sign in to comment.