From 9ca199aeeb3f86e1f619ab689aefa017ba0b9c42 Mon Sep 17 00:00:00 2001 From: Laszlo Balazs-Csiki Date: Wed, 21 Nov 2018 20:03:41 +0100 Subject: [PATCH] fix for issue #223 --- .../CheckThreadViolationRepaintManager.java | 18 +++++- ...ViolationRepaintManager_issue223_Test.java | 62 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 assertj-swing/src/test/java/org/assertj/swing/edt/FailOnThreadViolationRepaintManager_issue223_Test.java diff --git a/assertj-swing/src/main/java/org/assertj/swing/edt/CheckThreadViolationRepaintManager.java b/assertj-swing/src/main/java/org/assertj/swing/edt/CheckThreadViolationRepaintManager.java index c5eb9f3d..5c3078b5 100644 --- a/assertj-swing/src/main/java/org/assertj/swing/edt/CheckThreadViolationRepaintManager.java +++ b/assertj-swing/src/main/java/org/assertj/swing/edt/CheckThreadViolationRepaintManager.java @@ -37,6 +37,10 @@ * https://swinghelper.dev.java.net/ */ abstract class CheckThreadViolationRepaintManager extends RepaintManager { + // Should always be turned on because it shouldn't matter + // whether the component is showing (realized) or not. + // This flag exists only for historical reasons, see + // https://stackoverflow.com/questions/491323/is-it-safe-to-construct-swing-awt-widgets-not-on-the-event-dispatch-thread private final boolean completeCheck; private WeakReference lastComponent; @@ -62,14 +66,24 @@ public void addDirtyRegion(JComponent component, int x, int y, int w, int h) { super.addDirtyRegion(component, x, y, w, h); } + /** + * Rules enforced by this method: + * (1) it is always OK to reach this method on the Event Dispatch Thread. + * (2) it is generally not OK to reach this method outside the Event Dispatch Thread. + * (3) (exception form rule 2) except when we get here from a repaint() call, because repaint() is thread-safe + * (4) (exception from rule 3) it is not OK if swing code calls repaint() outside the EDT, because swing code should be called on the EDT. + * (5) (exception from rule 4) using SwingWorker subclasses should not be considered swing code. + */ private void checkThreadViolations(@Nonnull JComponent c) { if (!isEventDispatchThread() && (completeCheck || c.isShowing())) { boolean imageUpdate = false; boolean repaint = false; - boolean fromSwing = false; + boolean fromSwing = false; // whether we were in a swing method before before the repaint() call StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); for (StackTraceElement st : stackTrace) { - if (repaint && st.getClassName().startsWith("javax.swing.")) { + if (repaint + && st.getClassName().startsWith("javax.swing.") + && !st.getClassName().startsWith("javax.swing.SwingWorker")) { fromSwing = true; } if (repaint && "imageUpdate".equals(st.getMethodName())) { diff --git a/assertj-swing/src/test/java/org/assertj/swing/edt/FailOnThreadViolationRepaintManager_issue223_Test.java b/assertj-swing/src/test/java/org/assertj/swing/edt/FailOnThreadViolationRepaintManager_issue223_Test.java new file mode 100644 index 00000000..1d9fbfbc --- /dev/null +++ b/assertj-swing/src/test/java/org/assertj/swing/edt/FailOnThreadViolationRepaintManager_issue223_Test.java @@ -0,0 +1,62 @@ +/** + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * Copyright 2018 the original author or authors. + */ +package org.assertj.swing.edt; + +import org.junit.Test; + +import javax.swing.*; +import java.awt.EventQueue; +import java.lang.reflect.InvocationTargetException; +import java.util.concurrent.ExecutionException; + +import static org.junit.Assert.fail; + +/** + * See https://github.com/joel-costigliola/assertj-swing/issues/223 + */ +public class FailOnThreadViolationRepaintManager_issue223_Test { + /** + * This test fails with the original code, and passes with the fixed code + */ + @Test + public void calling_repaint_with_SwingWorker_should_not_be_EDT_Access_Violation() { + FailOnThreadViolationRepaintManager.install(); + + Runnable edtTask = () -> { + JLabel label = new JLabel("Test"); + SwingWorker notEDTTask = new SwingWorker() { + @Override + protected Object doInBackground() { + // it is OK to call repaint() from here + // because repaint() is thread-safe + label.repaint(); + return null; + } + }; + notEDTTask.execute(); + try { + // blocks until the doInBackground() method returns + notEDTTask.get(); + } catch (InterruptedException | ExecutionException e) { + e.printStackTrace(); + fail(); + } + }; + try { + EventQueue.invokeAndWait(edtTask); + } catch (InterruptedException | InvocationTargetException e) { + e.printStackTrace(); + fail(); + } + } +}