From 7ec08ed41c8f174f621aec0276fc865b4c4c6d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= Date: Wed, 27 Apr 2022 07:04:55 +0200 Subject: [PATCH] Allow the ProgressProvider to control the used instance of a monitor 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 https://github.com/eclipse-platform/eclipse.platform.runtime/issues/40 --- .../core/internal/jobs/JobManager.java | 33 ++++++++++------- .../core/runtime/jobs/ProgressProvider.java | 27 +++++++++++++- .../tests/runtime/jobs/BeginEndRuleTest.java | 36 ++++++++++++++++++- 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java index 9c6ec9abc..debdc2277 100644 --- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java +++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/JobManager.java @@ -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 @@ -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; @@ -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 monitor + * parameter. If no progress provider is set and monitor is null, a + * NullProgressMonitor is returned in all other cases the given + * monitor 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)); } } diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/ProgressProvider.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/ProgressProvider.java index a2239cef3..c572619f0 100644 --- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/ProgressProvider.java +++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/ProgressProvider.java @@ -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 @@ -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; @@ -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. + *

+ * The default implementation does the following: + *

+ * + * @param monitor the monitor to wrap, might be null + * @return a progress monitor for the given monitor but never + * null + * @since 3.13 + */ + public IProgressMonitor monitorFor(IProgressMonitor monitor) { + if (monitor == null || monitor instanceof NullProgressMonitor) { + return getDefaultMonitor(); + } + return monitor; + } } diff --git a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/BeginEndRuleTest.java b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/BeginEndRuleTest.java index d1f599449..b9dd6b784 100644 --- a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/BeginEndRuleTest.java +++ b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/BeginEndRuleTest.java @@ -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 @@ -50,6 +51,39 @@ public void run() { } } + @Test + public void testRuleCallsProgressProvider_monitorFor() { + AtomicBoolean createdMonitor = new AtomicBoolean(); + AtomicReference 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;