diff --git a/wpilibc/src/main/native/cpp/event/EventLoop.cpp b/wpilibc/src/main/native/cpp/event/EventLoop.cpp index 5af79c96d3..c85286ba37 100644 --- a/wpilibc/src/main/native/cpp/event/EventLoop.cpp +++ b/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 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& 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(); } diff --git a/wpilibc/src/main/native/include/frc/event/EventLoop.h b/wpilibc/src/main/native/include/frc/event/EventLoop.h index 224dd3b545..dca61220a5 100644 --- a/wpilibc/src/main/native/include/frc/event/EventLoop.h +++ b/wpilibc/src/main/native/include/frc/event/EventLoop.h @@ -38,5 +38,6 @@ class EventLoop { private: std::vector> m_bindings; + bool m_running{false}; }; } // namespace frc diff --git a/wpilibc/src/test/native/cpp/event/EventLoopTest.cpp b/wpilibc/src/test/native/cpp/event/EventLoopTest.cpp new file mode 100644 index 0000000000..2576a4655c --- /dev/null +++ b/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 + +#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(); +} diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/event/EventLoop.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/event/EventLoop.java index 3e220e06c2..90a5c44f96 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/event/EventLoop.java +++ b/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 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(); } } diff --git a/wpilibj/src/test/java/edu/wpi/first/wpilibj/event/EventLoopTest.java b/wpilibj/src/test/java/edu/wpi/first/wpilibj/event/EventLoopTest.java index ea3c569e65..644c18e15e 100644 --- a/wpilibj/src/test/java/edu/wpi/first/wpilibj/event/EventLoopTest.java +++ b/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(); + } }