Fix PIDControllerRunner member destruction order (#1801)

The mutexes in PIDControllerRunner are declared after the Notifier, and
when the PIDControllerRunner object is destructed, the member object
destructors are called in the reverse order in which they are declared.
The mutexes are destructed first, then the Notifier destructor is called
which stops the Notifier.

There's a window between those destructor calls during which the
Notifier can run the callable and attempt to lock a mutex that no longer
exists.

Declaring the Notifier after all the variables its callable uses fixes
this issue, as it ensures the Notifier is destructed first.
This commit is contained in:
Tyler Veness
2019-08-03 14:58:10 -07:00
committed by Peter Johnson
parent c98ca7310f
commit 8e93ce8929
2 changed files with 12 additions and 2 deletions

View File

@@ -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.
*