diff --git a/wpilibc/src/main/native/include/frc/controller/PIDControllerRunner.h b/wpilibc/src/main/native/include/frc/controller/PIDControllerRunner.h index aa47ca73a0..bcfacb9aab 100644 --- a/wpilibc/src/main/native/include/frc/controller/PIDControllerRunner.h +++ b/wpilibc/src/main/native/include/frc/controller/PIDControllerRunner.h @@ -55,7 +55,6 @@ class PIDControllerRunner : SendableBase { void InitSendable(SendableBuilder& builder) override; private: - Notifier m_notifier{&PIDControllerRunner::Run, this}; frc2::PIDController& m_controller; std::function m_measurementSource; std::function m_controllerOutput; @@ -67,6 +66,12 @@ class PIDControllerRunner : SendableBase { // Controller::Update() is already running at that time. mutable wpi::mutex m_outputMutex; + // This is declared after all other member variables so that during + // PIDControllerRunner destruction, the Notifier is stopped before any member + // variables its callable uses are destructed. This avoids use-after-free + // bugs like crashes when locking is attempted on deallocated mutexes. + Notifier m_notifier{&PIDControllerRunner::Run, this}; + void Run(); }; diff --git a/wpilibj/src/main/java/edu/wpi/first/wpilibj/controller/PIDControllerRunner.java b/wpilibj/src/main/java/edu/wpi/first/wpilibj/controller/PIDControllerRunner.java index 7d6edb866d..fa4b22a37c 100644 --- a/wpilibj/src/main/java/edu/wpi/first/wpilibj/controller/PIDControllerRunner.java +++ b/wpilibj/src/main/java/edu/wpi/first/wpilibj/controller/PIDControllerRunner.java @@ -16,7 +16,6 @@ import edu.wpi.first.wpilibj.SendableBase; import edu.wpi.first.wpilibj.smartdashboard.SendableBuilder; public class PIDControllerRunner extends SendableBase { - private final Notifier m_notifier = new Notifier(this::run); private final PIDController m_controller; private final DoubleConsumer m_controllerOutput; private final DoubleSupplier m_measurementSource; @@ -28,6 +27,12 @@ public class PIDControllerRunner extends SendableBase { // Controller.update() is already running at that time. private final ReentrantLock m_outputMutex = new ReentrantLock(); + // This is declared after all other member variables so that during + // PIDControllerRunner destruction, the Notifier is stopped before any member + // variables its callable uses are destructed. This avoids use-after-free + // bugs like crashes when locking is attempted on deallocated mutexes. + private final Notifier m_notifier = new Notifier(this::run); + /** * Allocates a PIDControllerRunner. *