Skip to content

Commit 2077a79

Browse files
laeubimickaelistria
authored andcommitted
Prevent JobManager UI freeze when waiting for a rule in the UI Thread
Fix eclipse-platform#8
1 parent 66106bc commit 2077a79

File tree

7 files changed

+215
-9
lines changed

7 files changed

+215
-9
lines changed

bundles/org.eclipse.e4.ui.progress/META-INF/MANIFEST.MF

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Manifest-Version: 1.0
22
Bundle-ManifestVersion: 2
33
Bundle-SymbolicName: org.eclipse.e4.ui.progress;singleton:=true
4-
Bundle-Version: 0.3.200.qualifier
4+
Bundle-Version: 0.3.400.qualifier
55
Bundle-Name: %pluginName
66
Bundle-Vendor: %providerName
77
Bundle-Localization: plugin
@@ -12,6 +12,7 @@ Require-Bundle: org.eclipse.core.jobs;bundle-version="3.5.400",
1212
org.eclipse.e4.core.di;bundle-version="1.3.0",
1313
org.eclipse.e4.ui.di;bundle-version="1.0.0",
1414
org.eclipse.jface;bundle-version="3.10.0",
15+
org.eclipse.core.jobs;bundle-version="3.13.0",
1516
org.eclipse.e4.ui.workbench,
1617
org.eclipse.e4.core.commands,
1718
org.eclipse.e4.core.services,

bundles/org.eclipse.e4.ui.progress/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
</parent>
1717
<groupId>org.eclipse.e4</groupId>
1818
<artifactId>org.eclipse.e4.ui.progress</artifactId>
19-
<version>0.3.200-SNAPSHOT</version>
19+
<version>0.3.400-SNAPSHOT</version>
2020
<packaging>eclipse-plugin</packaging>
2121

2222
<properties>

bundles/org.eclipse.e4.ui.progress/src/org/eclipse/e4/ui/progress/internal/ProgressManager.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2003, 2020 IBM Corporation and others.
2+
* Copyright (c) 2003, 2022 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -14,6 +14,7 @@
1414
* - Fix for Bug 151204 [Progress] Blocked status of jobs are not applied/reported
1515
* Lars Vogel <[email protected]> - Bug 422040
1616
* Philipp Bumann <[email protected]> - Bug 477602
17+
* Christoph Läubrich - Issue #8
1718
*******************************************************************************/
1819
package org.eclipse.e4.ui.progress.internal;
1920

@@ -35,7 +36,6 @@
3536
import org.eclipse.core.runtime.IProgressMonitor;
3637
import org.eclipse.core.runtime.IStatus;
3738
import org.eclipse.core.runtime.ListenerList;
38-
import org.eclipse.core.runtime.NullProgressMonitor;
3939
import org.eclipse.core.runtime.QualifiedName;
4040
import org.eclipse.core.runtime.Status;
4141
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
@@ -511,16 +511,21 @@ protected void sleepJobInfo(JobInfo info) {
511511

512512
@Override
513513
public IProgressMonitor getDefaultMonitor() {
514+
return monitorFor(null);
515+
}
516+
517+
@Override
518+
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
514519
// only need a default monitor for operations the UI thread
515520
// and only if there is a display
516521
Display display;
517522
if (PlatformUI.isWorkbenchRunning() && !PlatformUI.isWorkbenchStarting()) {
518523
display = getDisplay();
519524
if (!display.isDisposed() && (display.getThread() == Thread.currentThread())) {
520-
return new EventLoopProgressMonitor(new NullProgressMonitor());
525+
return new EventLoopProgressMonitor(IProgressMonitor.nullSafe(monitor));
521526
}
522527
}
523-
return super.getDefaultMonitor();
528+
return IProgressMonitor.nullSafe(monitor);
524529
}
525530

526531
/**

bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/ProgressManager.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2003, 2020 IBM Corporation and others.
2+
* Copyright (c) 2003, 2022 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -14,6 +14,7 @@
1414
* - Fix for Bug 151204 [Progress] Blocked status of jobs are not applied/reported
1515
* Lars Vogel <[email protected]> - Bug 422040
1616
* Terry Parker - Bug 454633, Report the cumulative error status of job groups in the ProgressManager
17+
* Christoph Läubrich - Issue #8
1718
*******************************************************************************/
1819
package org.eclipse.ui.internal.progress;
1920

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

576577
@Override
577578
public IProgressMonitor getDefaultMonitor() {
579+
return monitorFor(null);
580+
}
581+
582+
@Override
583+
public IProgressMonitor monitorFor(IProgressMonitor monitor) {
578584
// only need a default monitor for operations the UI thread
579585
// and only if there is a display
580586
Display display;
581587
if (PlatformUI.isWorkbenchRunning() && !PlatformUI.getWorkbench().isStarting()) {
582588
display = PlatformUI.getWorkbench().getDisplay();
583589
if (!display.isDisposed() && (display.getThread() == Thread.currentThread())) {
584-
return new EventLoopProgressMonitor(new NullProgressMonitor());
590+
return new EventLoopProgressMonitor(IProgressMonitor.nullSafe(monitor));
585591
}
586592
}
587-
return super.getDefaultMonitor();
593+
return IProgressMonitor.nullSafe(monitor);
588594
}
589595

590596
/**

bundles/org.eclipse.ui.workbench/META-INF/MANIFEST.MF

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.25.0,4.0.0)",
103103
org.eclipse.core.databinding.property;bundle-version="[1.2.0,2.0.0)",
104104
org.eclipse.core.databinding.observable;bundle-version="[1.2.0,2.0.0)",
105105
org.eclipse.core.expressions;bundle-version="[3.7.0,4.0.0)",
106+
org.eclipse.core.jobs;bundle-version="[3.13.0,4.0.0)",
106107
org.eclipse.e4.core.services;bundle-version="2.2.0",
107108
org.eclipse.e4.core.contexts;bundle-version="1.0.0",
108109
org.eclipse.e4.core.di;bundle-version="1.1.0",

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/ConcurrencyTestSuite.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
NestedSyncExecDeadlockTest.class,
2828
SyncExecWhileUIThreadWaitsForRuleTest.class,
2929
SyncExecWhileUIThreadWaitsForLock.class,
30+
NoFreezeWhileWaitingForRuleTest.class,
3031
TestBug105491.class,
3132
TestBug108162.class,
3233
TestBug98621.class,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2022 Christoph Läubrich and others.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Christoph Läubrich - initial API and implementation
13+
******************************************************************************/
14+
package org.eclipse.ui.tests.concurrency;
15+
16+
import static org.junit.Assert.assertFalse;
17+
import static org.junit.Assert.assertTrue;
18+
import static org.junit.Assert.fail;
19+
20+
import java.util.concurrent.CountDownLatch;
21+
import java.util.concurrent.TimeUnit;
22+
import java.util.concurrent.atomic.AtomicBoolean;
23+
24+
import org.eclipse.core.runtime.ICoreRunnable;
25+
import org.eclipse.core.runtime.IProgressMonitor;
26+
import org.eclipse.core.runtime.NullProgressMonitor;
27+
import org.eclipse.core.runtime.ProgressMonitorWrapper;
28+
import org.eclipse.core.runtime.jobs.Job;
29+
import org.eclipse.swt.widgets.Display;
30+
import org.eclipse.ui.tests.concurrency.SyncExecWhileUIThreadWaitsForRuleTest.TestRule;
31+
import org.junit.After;
32+
import org.junit.Before;
33+
import org.junit.Test;
34+
35+
/**
36+
* This test makes sure that a waiting on a rule do not blocks the UI thread, it
37+
* has three participants
38+
*
39+
* <ol>
40+
* <li>A Job that acquire and block a rule until it is notified</li>
41+
* <li>A runnable in the UI thread that tries to acquire the rule and will be
42+
* blocked</li>
43+
* <li>A thread that notify the first job via the UI thread to simulate some UI
44+
* events after a certain number of UI events have happened</li>
45+
* </ol>
46+
*
47+
*/
48+
public class NoFreezeWhileWaitingForRuleTest {
49+
50+
TestRule rule;
51+
IProgressMonitor ruleMonitor;
52+
private CountDownLatch eventQueueLatch;
53+
54+
@Before
55+
public void setUp() {
56+
rule = new SyncExecWhileUIThreadWaitsForRuleTest.TestRule();
57+
ruleMonitor = new ProgressMonitorWrapper(new NullProgressMonitor()) {
58+
AtomicBoolean canceled = new AtomicBoolean();
59+
60+
@Override
61+
public void setCanceled(boolean cancel) {
62+
canceled.set(cancel);
63+
}
64+
65+
@Override
66+
public boolean isCanceled() {
67+
return canceled.get();
68+
}
69+
};
70+
eventQueueLatch = new CountDownLatch(1);
71+
}
72+
73+
@After
74+
public void tearDown() {
75+
ruleMonitor.setCanceled(true); // just in case...
76+
}
77+
78+
@Test
79+
public void testWaiting() {
80+
// If you want to see the blocking uncomment the following line:
81+
// Job.getJobManager().setProgressProvider(null);
82+
Display display = Display.getDefault();
83+
assertTrue(display.getThread() == Thread.currentThread());
84+
try {
85+
Job blockingJob = spinRuleBlockingJob();
86+
CountDownLatch runnableLatch = spinUIblockingRunnable(display);
87+
Thread uiEventProducer = spinUIEventProducer(runnableLatch, display);
88+
while (!eventQueueLatch.await(10, TimeUnit.MILLISECONDS) && !ruleMonitor.isCanceled()) {
89+
while (display.readAndDispatch()) {
90+
}
91+
Thread.onSpinWait();
92+
}
93+
uiEventProducer.interrupt();
94+
blockingJob.join();
95+
} catch (InterruptedException e) {
96+
Thread.currentThread().interrupt();
97+
fail();
98+
}
99+
assertFalse("Timeout reached, blocking occurred!", ruleMonitor.isCanceled());
100+
}
101+
102+
/**
103+
* @param runnableLatch
104+
* @param display
105+
* @return
106+
*/
107+
private Thread spinUIEventProducer(CountDownLatch runnableLatch, Display display) {
108+
Thread thread = new Thread(() -> {
109+
// Stage 1: Wait for the UI-Thread to block...
110+
try {
111+
runnableLatch.await();
112+
} catch (InterruptedException e) {
113+
return;
114+
}
115+
// Stage 2: Schedule async exec to simulate some UI events happen...
116+
CountDownLatch eventLatch = new CountDownLatch(10);
117+
display.asyncExec(new Runnable() {
118+
@Override
119+
public void run() {
120+
if (ruleMonitor.isCanceled()) {
121+
// break out...
122+
return;
123+
}
124+
try {
125+
TimeUnit.MILLISECONDS.sleep(100);
126+
} catch (InterruptedException e) {
127+
}
128+
if (eventLatch.getCount() > 0) {
129+
eventLatch.countDown();
130+
display.asyncExec(this);
131+
}
132+
}
133+
});
134+
// Stage 3: wait for the events
135+
try {
136+
eventLatch.await();
137+
} catch (InterruptedException e) {
138+
return;
139+
}
140+
// Stage 4: signal the blocking thread...
141+
eventQueueLatch.countDown();
142+
});
143+
thread.setDaemon(true);
144+
thread.start();
145+
return thread;
146+
}
147+
148+
/**
149+
* @param display
150+
* @return
151+
*
152+
*/
153+
private CountDownLatch spinUIblockingRunnable(Display display) {
154+
CountDownLatch runnableRunning = new CountDownLatch(1);
155+
display.asyncExec(() -> {
156+
runnableRunning.countDown();
157+
Job.getJobManager().beginRule(rule, ruleMonitor);
158+
Job.getJobManager().endRule(rule);
159+
});
160+
return runnableRunning;
161+
}
162+
163+
private Job spinRuleBlockingJob() throws InterruptedException {
164+
CountDownLatch jobStarted = new CountDownLatch(1);
165+
long timeout = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(20);
166+
Job job = Job.create("Runs until notified", new ICoreRunnable() {
167+
168+
@Override
169+
public void run(IProgressMonitor monitor) {
170+
Job.getJobManager().beginRule(rule, ruleMonitor);
171+
jobStarted.countDown();
172+
try {
173+
while (!eventQueueLatch.await(1, TimeUnit.SECONDS)) {
174+
if (System.currentTimeMillis() > timeout) {
175+
ruleMonitor.setCanceled(true);
176+
break;
177+
}
178+
Thread.yield();
179+
}
180+
} catch (InterruptedException e) {
181+
Thread.currentThread().interrupt();
182+
} finally {
183+
Job.getJobManager().endRule(rule);
184+
}
185+
}
186+
});
187+
job.schedule();
188+
assertTrue("Job was not started", jobStarted.await(10, TimeUnit.SECONDS));
189+
return job;
190+
191+
}
192+
}

0 commit comments

Comments
 (0)