Skip to content

Commit 19f9144

Browse files
committed
Ensure all listeners are fired before analyzing jobs #195
Otherwise job.cancel(); job.schedule(); collector.isEmpty(); combination may produce false positives, as events take time to propagate.
1 parent bd4ac62 commit 19f9144

9 files changed

Lines changed: 72 additions & 82 deletions

File tree

runtime/contexts/org.eclipse.rcptt.ctx.debug.impl/src/org/eclipse/rcptt/debug/runtime/DebugContextProcessor.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,8 @@ public void apply(final Context ctx, BooleanSupplier isCancelled) throws CoreExc
233233
return;
234234
}
235235
long stop = currentTimeMillis() + TeslaLimits.getContextRunnableTimeout();
236-
final UIJobCollector collector = new UIJobCollector();
237-
Job.getJobManager().addJobChangeListener(collector);
238-
try {
236+
237+
try (UIJobCollector collector = new UIJobCollector(Job.getJobManager())) {
239238
UIRunnable.exec(new UIRunnable<Object>() {
240239
@Override
241240
public Object run() throws CoreException {
@@ -279,8 +278,6 @@ public Object run() throws CoreException {
279278
helper.applyLaunches(context.getLaunches());
280279
} catch (InterruptedException e) {
281280
throw new CoreException(RcpttPlugin.createStatus("Launch context was interrupted", e));
282-
} finally {
283-
Job.getJobManager().removeJobChangeListener(collector);
284281
}
285282
}
286283

runtime/contexts/org.eclipse.rcptt.ctx.preferences.impl/src/org/eclipse/rcptt/ctx/preferences/impl/PreferencesContextProcessor.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,8 @@ public class PreferencesContextProcessor implements IContextProcessor {
6060
@Override
6161
public void apply(final Context contextToApply, BooleanSupplier isCancelled) throws CoreException {
6262
long stop = currentTimeMillis() + TeslaLimits.getContextRunnableTimeout();
63-
final UIJobCollector collector = new UIJobCollector();
64-
Job.getJobManager().addJobChangeListener(collector);
6563
SWTUIPlayer.disableMessageDialogs();
66-
try {
64+
try (final UIJobCollector collector = new UIJobCollector(Job.getJobManager())) {
6765
UIRunnable.exec(new UIRunnable<Object>() {
6866
@Override
6967
public Object run() throws CoreException {
@@ -88,7 +86,6 @@ public Object run() throws CoreException {
8886
throw ee;
8987
} finally {
9088
SWTUIPlayer.enableMessageDialogs();
91-
Job.getJobManager().removeJobChangeListener(collector);
9289
}
9390
}
9491

runtime/contexts/org.eclipse.rcptt.ctx.resources.impl/src/org/eclipse/rcptt/ctx/resources/WorkspaceContextProcessor.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,7 @@ public void apply(final Context context, BooleanSupplier isCancelled) throws Cor
7575
final WorkspaceContext wc = (WorkspaceContext) context;
7676
long stop = currentTimeMillis() + TeslaLimits.getContextRunnableTimeout();
7777
// Smart cancel/close jobs with showed UI interactions.
78-
final UIJobCollector collector = new UIJobCollector();
79-
Job.getJobManager().addJobChangeListener(collector);
80-
81-
try {
78+
try (final UIJobCollector collector = new UIJobCollector(Job.getJobManager())){
8279
final IWorkspace ws = ResourcesPlugin.getWorkspace();
8380

8481
disableMessageDialogsAndEnableCollector(collector);
@@ -116,7 +113,6 @@ public Object run() throws CoreException {
116113
throw ee;
117114
} finally {
118115
SWTUIPlayer.enableMessageDialogs();
119-
Job.getJobManager().removeJobChangeListener(collector);
120116
}
121117
}
122118

runtime/contexts/org.eclipse.rcptt.ctx.workbench.impl/src/org/eclipse/rcptt/ctx/workbench/impl/WorkbenchContextProcessor.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,8 @@ public Object run() throws CoreException {
129129
@Override
130130
public void apply(final Context context, BooleanSupplier isCancelled) throws CoreException {
131131
final WorkbenchContext ctx = (WorkbenchContext) context;
132-
final UIJobCollector collector = new UIJobCollector();
133132
long stop = currentTimeMillis() + TeslaLimits.getContextRunnableTimeout();
134-
Job.getJobManager().addJobChangeListener(collector);
135-
try {
133+
try (final UIJobCollector collector = new UIJobCollector(Job.getJobManager())) {
136134
PlatformUI.getWorkbench().getDisplay().asyncExec(new Runnable() {
137135
@Override
138136
public void run() {
@@ -207,8 +205,6 @@ public Object run() throws CoreException {
207205
.createStatus("Failed to execute context: " + ctx.getName() + " Cause: " + e.getMessage(), e));
208206
RcpttPlugin.log(e);
209207
throw ee;
210-
} finally {
211-
Job.getJobManager().removeJobChangeListener(collector);
212208
}
213209
}
214210

runtime/ecl/org.eclipse.rcptt.tesla.ecl.impl/src/org/eclipse/rcptt/tesla/ecl/impl/UIRunnable.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,18 @@ public static <T> T exec(final UIRunnable<T> runnable) throws CoreException {
5757
public static <T> T exec(final UIRunnable<T> runnable, int timeout_ms, BooleanSupplier isCancelled) throws CoreException {
5858
final AtomicReference<RunningState> processed = new AtomicReference<RunningState>(RunningState.Starting);
5959
CompletableFuture<T> result = new CompletableFuture<T>();
60-
final UIJobCollector collector = new UIJobCollector();
6160
long start = System.currentTimeMillis();
6261
long stop = start + timeout_ms;
6362
long halfWay = start + (timeout_ms / 2);
6463
final Display display = PlatformUI.getWorkbench().getDisplay();
6564
if (Display.getCurrent() != null) {
6665
throw new IllegalStateException("Can't run in UI thread");
6766
}
68-
Job.getJobManager().addJobChangeListener(collector);
69-
collector.enable();
70-
final ITeslaEventListener listener = new ITeslaEventListener() {
67+
ITeslaEventListener listener = null;
68+
try (final UIJobCollector collector = new UIJobCollector(Job.getJobManager())) {
69+
collector.enable();
70+
71+
listener = new ITeslaEventListener() {
7172
@Override
7273
public synchronized boolean doProcessing(
7374
org.eclipse.rcptt.tesla.core.context.ContextManagement.Context currentContext) {
@@ -137,7 +138,7 @@ public void hasEvent(HasEventKind kind, String run) {
137138
}
138139
};
139140
final IStatus[] dialogCloseStatus = new IStatus[1];
140-
try {
141+
141142
TeslaEventManager.getManager().addEventListener(listener);
142143
while (!processed.get().equals(RunningState.Finished)) {
143144
if (display.isDisposed()) {
@@ -216,7 +217,6 @@ public void run() {
216217
throw new CoreException(createError(e));
217218
} finally {
218219
processed.set(RunningState.Done);
219-
Job.getJobManager().removeJobChangeListener(collector);
220220
TeslaEventManager.getManager().removeEventListener(listener);
221221
}
222222
}

runtime/tesla/org.eclipse.rcptt.tesla.swt/src/org/eclipse/rcptt/tesla/internal/ui/player/SWTUIPlayer.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,7 @@ protected SWTUIPlayer(Display display, Shell... ignoreWindows) {
228228
this.ignoreWindows[i++] = wrap(shell);
229229
}
230230
}
231-
collector = new UIJobCollector();
232-
Job.getJobManager().addJobChangeListener(collector);
231+
collector = new UIJobCollector(Job.getJobManager());
233232
timerListener = getTimerExecHelper();
234233
TeslaTimerExecManager.getManager().addEventListener(timerListener);
235234
}
@@ -2215,7 +2214,7 @@ public void clickAndWait(SWTUIElement w) {
22152214
}
22162215

22172216
private void shutdown() {
2218-
Job.getJobManager().removeJobChangeListener(collector);
2217+
collector.close();
22192218
BrowserManager.getInstance().clear();
22202219
TeslaTimerExecManager.getManager().removeEventListener(timerListener);
22212220
}

runtime/tesla/org.eclipse.rcptt.tesla.swt/src/org/eclipse/rcptt/tesla/internal/ui/player/UIJobCollector.java

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
import static java.util.Arrays.asList;
1414

15+
import java.io.Closeable;
16+
import java.io.IOException;
1517
import java.io.PrintWriter;
1618
import java.util.ArrayList;
1719
import java.util.Collection;
@@ -33,6 +35,7 @@
3335
import org.eclipse.core.runtime.Status;
3436
import org.eclipse.core.runtime.jobs.IJobChangeEvent;
3537
import org.eclipse.core.runtime.jobs.IJobChangeListener;
38+
import org.eclipse.core.runtime.jobs.IJobManager;
3639
import org.eclipse.core.runtime.jobs.ISchedulingRule;
3740
import org.eclipse.core.runtime.jobs.Job;
3841
import org.eclipse.rcptt.logging.Q7LoggingManager;
@@ -57,7 +60,7 @@
5760
/**
5861
* Manages jobs information and statuses.
5962
*/
60-
public class UIJobCollector implements IJobChangeListener {
63+
public class UIJobCollector implements Closeable {
6164
private static final boolean DEBUG = "true".equals(Platform.getDebugOption("org.eclipse.rcptt.tesla.swt/debug/jobCollector"));
6265
private static final boolean DEBUG_REPORT_OUTPUT = "true".equals(Platform.getDebugOption("org.eclipse.rcptt.tesla.swt/debug/debugReportOutput"));
6366
private static final PrintWriter DEBUG_WRITER = new PrintWriter(System.out);
@@ -246,6 +249,7 @@ public synchronized Thread getLastThread() {
246249
private boolean state;
247250
private boolean needDisable = false;
248251
private final IParameters parameters;
252+
private final IJobManager jobManager;
249253

250254

251255
private final Job removeCompletedJob = new Job("Eliminate completed jobs") {
@@ -331,6 +335,7 @@ private JobInfo getOrCreateJobInfo(Job job) {
331335
}
332336
}
333337

338+
private final IJobChangeListener jobListener = new IJobChangeListener() {
334339
@Override
335340
public void aboutToRun(IJobChangeEvent event) {
336341
}
@@ -399,7 +404,18 @@ public void scheduled(IJobChangeEvent event) {
399404
event("scheduled", jobInfo);
400405
jobInfo.poke();
401406
}
407+
@Override
408+
public void sleeping(IJobChangeEvent event) {
409+
Job job = event.getJob();
410+
if (job.belongsTo(FAMILY)) {
411+
return;
412+
}
413+
JobInfo info = getOrCreateJobInfo(job);
414+
info.sleeping();
415+
event("sleeping", info);
416+
}
402417

418+
};
403419
protected JobStatus calcJobStatus(Job job) {
404420
JobStatus result = detectJobStatus(job);
405421
if (result == null) {
@@ -468,7 +484,7 @@ public static JobStatus detectJobStatus(Job job) {
468484
return status;
469485
}
470486

471-
public UIJobCollector() {
487+
public UIJobCollector(IJobManager jobManager) {
472488
this(new IParameters() {
473489
@Override
474490
public int stepInterval() {
@@ -494,24 +510,16 @@ public int timeout() {
494510
public int delayToWaitFor() {
495511
return TeslaLimits.getJobWaitForDelayedTimeout();
496512
}
497-
});
513+
}, jobManager);
498514
}
499515

500-
public UIJobCollector(IParameters parameters) {
516+
public UIJobCollector(IParameters parameters, IJobManager jobManager) {
501517
if (parameters == null)
502518
throw new NullPointerException();
503519
this.parameters = new NormalizedParameters(parameters);
504-
}
505-
506-
@Override
507-
public void sleeping(IJobChangeEvent event) {
508-
Job job = event.getJob();
509-
if (job.belongsTo(FAMILY)) {
510-
return;
511-
}
512-
JobInfo info = getOrCreateJobInfo(job);
513-
info.sleeping();
514-
event("sleeping", info);
520+
this.jobManager = jobManager;
521+
jobManager.addJobChangeListener(jobListener);
522+
515523
}
516524

517525
private static boolean isModal(Shell shell) {
@@ -892,6 +900,9 @@ private boolean isBlocked(Job job) {
892900
}
893901

894902
private boolean removeCompletedJobs() {
903+
for (Job j: jobManager.find(null)) {
904+
TeslaSWTAccess.waitListeners(j); // force jobs being scheduled to invoke their listeners
905+
}
895906
try {
896907
if (!removeCompletedJob.join(1, null)) {
897908
return false;
@@ -1103,4 +1114,10 @@ private void event(String message, JobInfo job) {
11031114
private static boolean shouldDebug(Job job) {
11041115
return DEBUG && !IGNORED_BY_DEFAULT.contains(job.getClass().getName());
11051116
}
1117+
1118+
@Override
1119+
public void close() {
1120+
disable();
1121+
jobManager.removeJobChangeListener(jobListener);
1122+
}
11061123
}

runtime/tesla/org.eclipse.rcptt.tesla.ui.ide/src/org/eclipse/rcptt/tesla/internal/ui/problemview/ProblemViewSupportProcessor.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public void postSelect(Element element, IElementProcessorMapper mapper) {
131131
}
132132

133133
private class WaitForJobsStatus extends PreExecuteStatus {
134-
final UIJobCollector collector = new UIJobCollector() {
134+
final UIJobCollector collector = new UIJobCollector(Job.getJobManager()) {
135135
@Override
136136
protected JobStatus calcJobStatus(Job job) {
137137
if (isMarkersJob(job)) {
@@ -156,8 +156,7 @@ protected boolean isSyncSupported() {
156156

157157
@Override
158158
public void clean() {
159-
collector.disable();
160-
Job.getJobManager().removeJobChangeListener(collector);
159+
collector.close();
161160
}
162161

163162
public WaitForJobsStatus(boolean canExecute) {
@@ -193,7 +192,7 @@ public PreExecuteStatus preExecute(Command command,
193192
return s;
194193
}
195194
s.collector.disable();
196-
Job.getJobManager().removeJobChangeListener(s.collector);
195+
s.clean();
197196
}
198197
// Check for marker update job is present
199198
Job[] find = Job.getJobManager().find(null);
@@ -203,7 +202,6 @@ public PreExecuteStatus preExecute(Command command,
203202
st.collector.disable();
204203
return null;
205204
}
206-
Job.getJobManager().addJobChangeListener(st.collector);
207205
Q7WaitUtils.updateInfo("problems.view", "wait", info);
208206
return st;
209207
}

0 commit comments

Comments
 (0)