[wpilib] Throw early when EventLoop is modified while running (#6115)

wpilibc/src/main/native/cpp/event/EventLoop.cpp

@@ -4,20 +4,41 @@
 
 #include "frc/event/EventLoop.h"
 
+#include "frc/Errors.h"
+
 using namespace frc;
 
+namespace {
+struct RunningSetter {
+  bool& m_running;
+  explicit RunningSetter(bool& running) noexcept : m_running{running} {
+    m_running = true;
+  }
+  ~RunningSetter() noexcept { m_running = false; }
+};
+}  // namespace
+
 EventLoop::EventLoop() {}
 
 void EventLoop::Bind(wpi::unique_function<void()> action) {
+  if (m_running) {
+    throw FRC_MakeError(err::Error,
+                        "Cannot bind EventLoop while it is running");
+  }
   m_bindings.emplace_back(std::move(action));
 }
 
 void EventLoop::Poll() {
+  RunningSetter runSetter{m_running};
   for (wpi::unique_function<void()>& action : m_bindings) {
     action();
   }
 }
 
 void EventLoop::Clear() {
+  if (m_running) {
+    throw FRC_MakeError(err::Error,
+                        "Cannot clear EventLoop while it is running");
+  }
   m_bindings.clear();
 }

wpilibc/src/main/native/include/frc/event/EventLoop.h

@@ -38,5 +38,6 @@ class EventLoop {
 
  private:
   std::vector<wpi::unique_function<void()>> m_bindings;
+  bool m_running{false};
 };
 }  // namespace frc

wpilibc/src/test/native/cpp/event/EventLoopTest.cpp

@@ -0,0 +1,24 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#include <gtest/gtest.h>
+
+#include "frc/Errors.h"
+#include "frc/event/EventLoop.h"
+
+using namespace frc;
+
+TEST(EventLoopTest, ConcurrentModification) {
+  EventLoop loop;
+
+  loop.Bind([&loop] { ASSERT_THROW(loop.Bind([] {}), frc::RuntimeError); });
+
+  loop.Poll();
+
+  loop.Clear();
+
+  loop.Bind([&loop] { ASSERT_THROW(loop.Clear(), frc::RuntimeError); });
+
+  loop.Poll();
+}

wpilibj/src/main/java/edu/wpi/first/wpilibj/event/EventLoop.java

@@ -5,6 +5,7 @@
 package edu.wpi.first.wpilibj.event;
 
 import java.util.Collection;
+import java.util.ConcurrentModificationException;
 import java.util.LinkedHashSet;
 
 /**
@@ -12,6 +13,7 @@ import java.util.LinkedHashSet;
  */
 public final class EventLoop {
   private final Collection<Runnable> m_bindings = new LinkedHashSet<>();
+  private boolean m_running;
 
   /**
    * Bind a new action to run when the loop is polled.
@@ -19,16 +21,27 @@ public final class EventLoop {
    * @param action the action to run.
    */
   public void bind(Runnable action) {
+    if (m_running) {
+      throw new ConcurrentModificationException("Cannot bind EventLoop while it is running");
+    }
     m_bindings.add(action);
   }
 
   /** Poll all bindings. */
   public void poll() {
-    m_bindings.forEach(Runnable::run);
+    try {
+      m_running = true;
+      m_bindings.forEach(Runnable::run);
+    } finally {
+      m_running = false;
+    }
   }
 
   /** Clear all bindings. */
   public void clear() {
+    if (m_running) {
+      throw new ConcurrentModificationException("Cannot clear EventLoop while it is running");
+    }
     m_bindings.clear();
   }
 }

wpilibj/src/test/java/edu/wpi/first/wpilibj/event/EventLoopTest.java

@@ -5,7 +5,9 @@
 package edu.wpi.first.wpilibj.event;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.util.ConcurrentModificationException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.jupiter.api.Test;
@@ -58,4 +60,33 @@ class EventLoopTest {
     // shouldn't change
     assertEquals(1, counter.get());
   }
+
+  @Test
+  void testConcurrentModification() {
+    var loop = new EventLoop();
+
+    loop.bind(
+        () -> {
+          assertThrows(
+              ConcurrentModificationException.class,
+              () -> {
+                loop.bind(() -> {});
+              });
+        });
+
+    loop.poll();
+
+    loop.clear();
+
+    loop.bind(
+        () -> {
+          assertThrows(
+              ConcurrentModificationException.class,
+              () -> {
+                loop.clear();
+              });
+        });
+
+    loop.poll();
+  }
 }