From 9b24e9f9ebffadfa7d715b457b9fb1b8d51c3bdb Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 20 Mar 2018 18:06:03 -0700 Subject: [PATCH] Use ConcurrentLinkedDeque for EventHandler ArrayList::add is not thread safe. I ran into cases where async tests using utests would fail even when all of the individual tests passed. This was because multiple threads called back into the handle method of the handler instance variable, which just delegated to eventList::add. When this happened, one of the events would get added to the list as a null reference, which would manifest as an NPE upstream on the master process. After this change, my tests stopped failing. --- testing/agent/src/main/java/sbt/ForkMain.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testing/agent/src/main/java/sbt/ForkMain.java b/testing/agent/src/main/java/sbt/ForkMain.java index 24acaf52b..a92d02506 100644 --- a/testing/agent/src/main/java/sbt/ForkMain.java +++ b/testing/agent/src/main/java/sbt/ForkMain.java @@ -17,6 +17,7 @@ import java.net.Socket; import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.concurrent.*; @@ -294,7 +295,7 @@ final public class ForkMain { Task[] nestedTasks; final TaskDef taskDef = task.taskDef(); try { - final List eventList = new ArrayList(); + final Collection eventList = new ConcurrentLinkedDeque(); final EventHandler handler = new EventHandler() { public void handle(final Event e){ eventList.add(new ForkEvent(e)); } }; logDebug(os, " Running " + taskDef); nestedTasks = task.execute(handler, loggers);