Skip to content

Commit

Permalink
Prevent JobManager UI freeze when waiting for a rule in the UI Thread
Browse files Browse the repository at this point in the history
  • Loading branch information
laeubi committed May 2, 2022
1 parent 66106bc commit 2822bb0
Show file tree
Hide file tree
Showing 7 changed files with 215 additions and 9 deletions.
3 changes: 2 additions & 1 deletion bundles/org.eclipse.e4.ui.progress/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: org.eclipse.e4.ui.progress;singleton:=true
Bundle-Version: 0.3.200.qualifier
Bundle-Version: 0.3.400.qualifier
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand All @@ -12,6 +12,7 @@ Require-Bundle: org.eclipse.core.jobs;bundle-version="3.5.400",
org.eclipse.e4.core.di;bundle-version="1.3.0",
org.eclipse.e4.ui.di;bundle-version="1.0.0",
org.eclipse.jface;bundle-version="3.10.0",
org.eclipse.core.jobs;bundle-version="3.13.0",
org.eclipse.e4.ui.workbench,
org.eclipse.e4.core.commands,
org.eclipse.e4.core.services,
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.e4.ui.progress/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
</parent>
<groupId>org.eclipse.e4</groupId>
<artifactId>org.eclipse.e4.ui.progress</artifactId>
<version>0.3.200-SNAPSHOT</version>
<version>0.3.300-SNAPSHOT</version>
<packaging>eclipse-plugin</packaging>

<properties>
Expand Down
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 @@ -14,6 +14,7 @@
* - Fix for Bug 151204 [Progress] Blocked status of jobs are not applied/reported
* Lars Vogel <Lars.Vogel@gmail.com> - Bug 422040
* Philipp Bumann <bumannp@gmail.com> - Bug 477602
* Christoph Läubrich - Issue #8
*******************************************************************************/
package org.eclipse.e4.ui.progress.internal;

Expand All @@ -35,7 +36,6 @@
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.ListenerList;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.QualifiedName;
import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
Expand Down Expand Up @@ -511,16 +511,21 @@ protected void sleepJobInfo(JobInfo info) {

@Override
public IProgressMonitor getDefaultMonitor() {
return monitorFor(null);
}

@Override
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
// only need a default monitor for operations the UI thread
// and only if there is a display
Display display;
if (PlatformUI.isWorkbenchRunning() && !PlatformUI.isWorkbenchStarting()) {
display = getDisplay();
if (!display.isDisposed() && (display.getThread() == Thread.currentThread())) {
return new EventLoopProgressMonitor(new NullProgressMonitor());
return new EventLoopProgressMonitor(IProgressMonitor.nullSafe(monitor));
}
}
return super.getDefaultMonitor();
return IProgressMonitor.nullSafe(monitor);
}

/**
Expand Down
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 @@ -14,6 +14,7 @@
* - Fix for Bug 151204 [Progress] Blocked status of jobs are not applied/reported
* Lars Vogel <Lars.Vogel@gmail.com> - Bug 422040
* Terry Parker - Bug 454633, Report the cumulative error status of job groups in the ProgressManager
* Christoph Läubrich - Issue #8
*******************************************************************************/
package org.eclipse.ui.internal.progress;

Expand Down Expand Up @@ -575,16 +576,21 @@ public IProgressMonitor createMonitor(Job job) {

@Override
public IProgressMonitor getDefaultMonitor() {
return monitorFor(null);
}

@Override
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
// only need a default monitor for operations the UI thread
// and only if there is a display
Display display;
if (PlatformUI.isWorkbenchRunning() && !PlatformUI.getWorkbench().isStarting()) {
display = PlatformUI.getWorkbench().getDisplay();
if (!display.isDisposed() && (display.getThread() == Thread.currentThread())) {
return new EventLoopProgressMonitor(new NullProgressMonitor());
return new EventLoopProgressMonitor(IProgressMonitor.nullSafe(monitor));
}
}
return super.getDefaultMonitor();
return IProgressMonitor.nullSafe(monitor);
}

/**
Expand Down
1 change: 1 addition & 0 deletions bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.25.0,4.0.0)",
org.eclipse.core.databinding.property;bundle-version="[1.2.0,2.0.0)",
org.eclipse.core.databinding.observable;bundle-version="[1.2.0,2.0.0)",
org.eclipse.core.expressions;bundle-version="[3.7.0,4.0.0)",
org.eclipse.core.jobs;bundle-version="[3.13.0,4.0.0)",
org.eclipse.e4.core.services;bundle-version="2.2.0",
org.eclipse.e4.core.contexts;bundle-version="1.0.0",
org.eclipse.e4.core.di;bundle-version="1.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
NestedSyncExecDeadlockTest.class,
SyncExecWhileUIThreadWaitsForRuleTest.class,
SyncExecWhileUIThreadWaitsForLock.class,
NoFreezeWhileWaitingForRuleTest.class,
TestBug105491.class,
TestBug108162.class,
TestBug98621.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
/*******************************************************************************
* Copyright (c) 2022 Christoph Läubrich and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Christoph Läubrich - initial API and implementation
******************************************************************************/
package org.eclipse.ui.tests.concurrency;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;

import org.eclipse.core.runtime.ICoreRunnable;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.ProgressMonitorWrapper;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.tests.concurrency.SyncExecWhileUIThreadWaitsForRuleTest.TestRule;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

/**
* This test makes sure that a waiting on a rule do not blocks the UI thread, it
* has three participants
*
* <ol>
* <li>A Job that acquire and block a rule until it is notified</li>
* <li>A runnable in the UI thread that tries to acquire the rule and will be
* blocked</li>
* <li>A thread that notify the first job via the UI thread to simulate some UI
* events after a certain number of UI events have happened</li>
* </ol>
*
*/
public class NoFreezeWhileWaitingForRuleTest {

TestRule rule;
IProgressMonitor ruleMonitor;
private CountDownLatch eventQueueLatch;

@Before
public void setUp() {
rule = new SyncExecWhileUIThreadWaitsForRuleTest.TestRule();
ruleMonitor = new ProgressMonitorWrapper(new NullProgressMonitor()) {
AtomicBoolean canceled = new AtomicBoolean();

@Override
public void setCanceled(boolean cancel) {
canceled.set(cancel);
}

@Override
public boolean isCanceled() {
return canceled.get();
}
};
eventQueueLatch = new CountDownLatch(1);
}

@After
public void tearDown() {
ruleMonitor.setCanceled(true); // just in case...
}

@Test
public void testWaiting() {
// If you want to see the blocking uncomment the following line:
// Job.getJobManager().setProgressProvider(null);
Display display = Display.getDefault();
assertTrue(display.getThread() == Thread.currentThread());
try {
Job blockingJob = spinRuleBlockingJob();
CountDownLatch runnableLatch = spinUIblockingRunnable(display);
Thread uiEventProducer = spinUIEventProducer(runnableLatch, display);
while (!eventQueueLatch.await(10, TimeUnit.MILLISECONDS) && !ruleMonitor.isCanceled()) {
while (display.readAndDispatch()) {
}
Thread.onSpinWait();
}
uiEventProducer.interrupt();
blockingJob.join();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
fail();
}
assertFalse("Timeout reached, blocking occurred!", ruleMonitor.isCanceled());
}

/**
* @param runnableLatch
* @param display
* @return
*/
private Thread spinUIEventProducer(CountDownLatch runnableLatch, Display display) {
Thread thread = new Thread(() -> {
// Stage 1: Wait for the UI-Thread to block...
try {
runnableLatch.await();
} catch (InterruptedException e) {
return;
}
// Stage 2: Schedule async exec to simulate some UI events happen...
CountDownLatch eventLatch = new CountDownLatch(10);
display.asyncExec(new Runnable() {
@Override
public void run() {
if (ruleMonitor.isCanceled()) {
// break out...
return;
}
try {
TimeUnit.MILLISECONDS.sleep(100);
} catch (InterruptedException e) {
}
if (eventLatch.getCount() > 0) {
eventLatch.countDown();
display.asyncExec(this);
}
}
});
// Stage 3: wait for the events
try {
eventLatch.await();
} catch (InterruptedException e) {
return;
}
// Stage 4: signal the blocking thread...
eventQueueLatch.countDown();
});
thread.setDaemon(true);
thread.start();
return thread;
}

/**
* @param display
* @return
*
*/
private CountDownLatch spinUIblockingRunnable(Display display) {
CountDownLatch runnableRunning = new CountDownLatch(1);
display.asyncExec(() -> {
runnableRunning.countDown();
Job.getJobManager().beginRule(rule, ruleMonitor);
Job.getJobManager().endRule(rule);
});
return runnableRunning;
}

private Job spinRuleBlockingJob() throws InterruptedException {
CountDownLatch jobStarted = new CountDownLatch(1);
long timeout = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(20);
Job job = Job.create("Runs until notified", new ICoreRunnable() {

@Override
public void run(IProgressMonitor monitor) {
Job.getJobManager().beginRule(rule, ruleMonitor);
jobStarted.countDown();
try {
while (!eventQueueLatch.await(1, TimeUnit.SECONDS)) {
if (System.currentTimeMillis() > timeout) {
ruleMonitor.setCanceled(true);
break;
}
Thread.yield();
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} finally {
Job.getJobManager().endRule(rule);
}
}
});
job.schedule();
assertTrue("Job was not started", jobStarted.await(10, TimeUnit.SECONDS));
return job;

}
}

0 comments on commit 2822bb0

Please sign in to comment.