From 58a122ee28f893d2809a5dd116509b534561fdd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Mon, 7 Feb 2022 15:07:47 +0100 Subject: [PATCH 1/7] Add test that checks exception handling on background threads --- .../bramble/test/ThreadExceptionTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java new file mode 100644 index 000000000..6491df41a --- /dev/null +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java @@ -0,0 +1,32 @@ +package org.briarproject.bramble.test; + +import org.junit.Test; + +import static org.junit.Assert.fail; + +public class ThreadExceptionTest extends BrambleTestCase { + + @Test(expected = AssertionError.class) + public void testAssertionErrorMakesTestCaseFail() { + // This is what BrambleTestCase does, too: + fail(); + } + + @Test(expected = AssertionError.class) + public void testExceptionInThreadMakesTestCaseFail() { + Thread t = new Thread(() -> { + System.out.println("thread before exception"); + throw new RuntimeException("boom"); + }); + + t.start(); + try { + t.join(); + System.out.println("joined thread"); + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + + +} From e3f2a3012087e70ac4e7890c11b0f1efbf6f9501 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Mon, 7 Feb 2022 17:26:26 +0100 Subject: [PATCH 2/7] Make BrambleTestCase fail if background thread throws an exception --- .../bramble/test/BrambleTestCase.java | 19 ++++++++++++++++++- .../bramble/test/ThreadExceptionTest.java | 5 +++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java index 9fae78043..279c35d70 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java @@ -1,17 +1,34 @@ package org.briarproject.bramble.test; +import org.junit.After; +import org.junit.Before; + import java.lang.Thread.UncaughtExceptionHandler; import static org.junit.Assert.fail; public abstract class BrambleTestCase { + private volatile boolean exceptionInBackgroundThread = false; + public BrambleTestCase() { // Ensure exceptions thrown on worker threads cause tests to fail UncaughtExceptionHandler fail = (thread, throwable) -> { throwable.printStackTrace(); - fail(); + exceptionInBackgroundThread = true; }; Thread.setDefaultUncaughtExceptionHandler(fail); } + + @Before + public void before() { + exceptionInBackgroundThread = false; + } + + @After + public void after() { + if (exceptionInBackgroundThread) { + fail("background thread has thrown an exception"); + } + } } diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java index 6491df41a..184c057d8 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java @@ -12,7 +12,7 @@ public class ThreadExceptionTest extends BrambleTestCase { fail(); } - @Test(expected = AssertionError.class) + @Test public void testExceptionInThreadMakesTestCaseFail() { Thread t = new Thread(() -> { System.out.println("thread before exception"); @@ -24,7 +24,8 @@ public class ThreadExceptionTest extends BrambleTestCase { t.join(); System.out.println("joined thread"); } catch (InterruptedException e) { - e.printStackTrace(); + System.out.println("interrupted while joining thread"); + fail(); } } From 32b62d3e30aa43ed6a9f9cfd8fa8cc71002acf20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Fri, 4 Mar 2022 07:57:50 +0100 Subject: [PATCH 3/7] Allow BrambleTestCase to handle background thread exceptions gracefully during after() --- .../bramble/test/BrambleTestCase.java | 16 +++++++++++----- .../bramble/test/ThreadExceptionTest.java | 5 ++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java index 279c35d70..c2d90f37e 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java @@ -4,31 +4,37 @@ import org.junit.After; import org.junit.Before; import java.lang.Thread.UncaughtExceptionHandler; +import java.util.logging.Level; +import java.util.logging.Logger; +import static java.util.logging.Logger.getLogger; import static org.junit.Assert.fail; public abstract class BrambleTestCase { - private volatile boolean exceptionInBackgroundThread = false; + private static final Logger LOG = + getLogger(BrambleTestCase.class.getName()); + + protected volatile boolean exceptionInBackgroundThread = false; public BrambleTestCase() { // Ensure exceptions thrown on worker threads cause tests to fail UncaughtExceptionHandler fail = (thread, throwable) -> { - throwable.printStackTrace(); + LOG.log(Level.WARNING, "Caught unhandled exception", throwable); exceptionInBackgroundThread = true; }; Thread.setDefaultUncaughtExceptionHandler(fail); } @Before - public void before() { + public void beforeBrambleTestCase() { exceptionInBackgroundThread = false; } @After - public void after() { + public void afterBrambleTestCase() { if (exceptionInBackgroundThread) { - fail("background thread has thrown an exception"); + fail("background thread has thrown an exception unexpectedly"); } } } diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java index 184c057d8..464f9df0d 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java @@ -1,5 +1,6 @@ package org.briarproject.bramble.test; +import org.junit.Assert; import org.junit.Test; import static org.junit.Assert.fail; @@ -27,7 +28,9 @@ public class ThreadExceptionTest extends BrambleTestCase { System.out.println("interrupted while joining thread"); fail(); } + + Assert.assertTrue(exceptionInBackgroundThread); + exceptionInBackgroundThread = false; } - } From 288f3331ec26abcfad1ff9f289d547add5e086f2 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 28 Mar 2022 14:59:01 +0100 Subject: [PATCH 4/7] Include background exception in test failure report. --- .../bramble/test/BrambleTestCase.java | 17 +++++++++++------ .../bramble/test/ThreadExceptionTest.java | 6 +++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java index c2d90f37e..bac2196f3 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java @@ -7,34 +7,39 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.Nullable; + import static java.util.logging.Logger.getLogger; -import static org.junit.Assert.fail; public abstract class BrambleTestCase { private static final Logger LOG = getLogger(BrambleTestCase.class.getName()); - protected volatile boolean exceptionInBackgroundThread = false; + @Nullable + protected volatile Throwable exceptionInBackgroundThread = null; public BrambleTestCase() { // Ensure exceptions thrown on worker threads cause tests to fail UncaughtExceptionHandler fail = (thread, throwable) -> { LOG.log(Level.WARNING, "Caught unhandled exception", throwable); - exceptionInBackgroundThread = true; + exceptionInBackgroundThread = throwable; }; Thread.setDefaultUncaughtExceptionHandler(fail); } @Before public void beforeBrambleTestCase() { - exceptionInBackgroundThread = false; + exceptionInBackgroundThread = null; } @After public void afterBrambleTestCase() { - if (exceptionInBackgroundThread) { - fail("background thread has thrown an exception unexpectedly"); + Throwable thrown = exceptionInBackgroundThread; + if (thrown != null) { + throw new AssertionError( + "background thread has thrown an exception unexpectedly", + thrown); } } } diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java index 464f9df0d..14a5d21bb 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/ThreadExceptionTest.java @@ -1,8 +1,8 @@ package org.briarproject.bramble.test; -import org.junit.Assert; import org.junit.Test; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; public class ThreadExceptionTest extends BrambleTestCase { @@ -29,8 +29,8 @@ public class ThreadExceptionTest extends BrambleTestCase { fail(); } - Assert.assertTrue(exceptionInBackgroundThread); - exceptionInBackgroundThread = false; + assertNotNull(exceptionInBackgroundThread); + exceptionInBackgroundThread = null; } } From 3d6972fd737d4eab369e2e2bbd98b503a746aecb Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 28 Mar 2022 14:59:43 +0100 Subject: [PATCH 5/7] Fix race condition in IntroductionIntegrationTest. --- .../briar/introduction/IntroductionIntegrationTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index 952576c98..29131fe8b 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -986,6 +986,10 @@ public class IntroductionIntegrationTest @Test public void testIntroduceesRemovedCleanup() throws Exception { addListeners(true, true); + // The second introducee shouldn't respond to the introduction + // otherwise there would be a race between the response to the REQUEST + // and the delivery of the ABORT + listener2.answerRequests = false; // make introduction introductionManager0 From 3e597ceff85242173c506ea80cd2bd2da06978c9 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 28 Mar 2022 15:09:26 +0100 Subject: [PATCH 6/7] Use a constructor that Animal Sniffer knows about. --- .../java/org/briarproject/bramble/test/BrambleTestCase.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java index bac2196f3..dd578c67c 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java @@ -36,10 +36,6 @@ public abstract class BrambleTestCase { @After public void afterBrambleTestCase() { Throwable thrown = exceptionInBackgroundThread; - if (thrown != null) { - throw new AssertionError( - "background thread has thrown an exception unexpectedly", - thrown); - } + if (thrown != null) throw new AssertionError(thrown); } } From 243df3096a841e7e3fd8dbc7ac7dea1f78cd2224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20K=C3=BCrten?= Date: Tue, 29 Mar 2022 09:30:51 +0200 Subject: [PATCH 7/7] Add logging message when BrambleTestCase detects background thread exception --- .../test/java/org/briarproject/bramble/test/BrambleTestCase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java index dd578c67c..9ac8a7a40 100644 --- a/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java +++ b/bramble-api/src/test/java/org/briarproject/bramble/test/BrambleTestCase.java @@ -36,6 +36,7 @@ public abstract class BrambleTestCase { @After public void afterBrambleTestCase() { Throwable thrown = exceptionInBackgroundThread; + LOG.warning("background thread has thrown an exception unexpectedly"); if (thrown != null) throw new AssertionError(thrown); } }