fixed various memory leaks/overruns/etc
I found many issues using AddressSanitizer and LeakSanitizer.
diff --git a/aos/common/queue_types.h b/aos/common/queue_types.h
index eb0527e..7381029 100644
--- a/aos/common/queue_types.h
+++ b/aos/common/queue_types.h
@@ -40,6 +40,7 @@
for (int i = 0; i < number_fields; ++i) {
delete fields[i];
}
+ delete[] fields;
}
// Returns -1 if max_bytes is too small.
diff --git a/bbb_cape/src/bbb/cows_test.cc b/bbb_cape/src/bbb/cows_test.cc
index 06eba5b..e75af6b 100644
--- a/bbb_cape/src/bbb/cows_test.cc
+++ b/bbb_cape/src/bbb/cows_test.cc
@@ -7,8 +7,8 @@
TEST(CowsTest, StupidZeros) {
static const uint8_t kTestInput[] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x08, 0x01,
0x00};
- uint32_t input[3];
- memcpy(input, kTestInput, 12);
+ uint32_t input[2];
+ memcpy(input, kTestInput, 8);
uint32_t output[2];
EXPECT_EQ(
1u, cows_unstuff(input, sizeof(kTestInput), output, sizeof(output) * 4));
diff --git a/bbb_cape/src/bbb/packet_finder.cc b/bbb_cape/src/bbb/packet_finder.cc
index 783ce32..43363b8 100644
--- a/bbb_cape/src/bbb/packet_finder.cc
+++ b/bbb_cape/src/bbb/packet_finder.cc
@@ -36,8 +36,8 @@
}
PacketFinder::~PacketFinder() {
- delete buf_;
- delete unstuffed_data_;
+ delete[] buf_;
+ delete[] unstuffed_data_;
}
bool PacketFinder::FindPacket(const ::Time &timeout_time) {
diff --git a/bbb_cape/src/bbb/packet_finder_test.cc b/bbb_cape/src/bbb/packet_finder_test.cc
index dddf0ec..b4937b1 100644
--- a/bbb_cape/src/bbb/packet_finder_test.cc
+++ b/bbb_cape/src/bbb/packet_finder_test.cc
@@ -58,7 +58,9 @@
}
EXPECT_EQ(!expect_failure,
packet_finder.ReadPacket(::aos::time::Time(0, 0)));
- if (expect_failure && i + 1 != *failure && i + 1 != packets) {
+ if (expect_failure &&
+ (failure == expected_failures.end() || i + 1 != *failure) &&
+ i + 1 != packets) {
int failures = 0;
while (!packet_finder.ReadPacket(::aos::time::Time(0, 0))) {
++failures;
diff --git a/frc971/constants.cc b/frc971/constants.cc
index dd8583d..5be9b7e 100755
--- a/frc971/constants.cc
+++ b/frc971/constants.cc
@@ -4,9 +4,16 @@
#include <stdint.h>
#include <inttypes.h>
+#include <map>
+
+#if __has_feature(address_sanitizer)
+#include "sanitizer/lsan_interface.h"
+#endif
+
#include "aos/common/logging/logging.h"
#include "aos/common/once.h"
#include "aos/common/network/team_number.h"
+#include "aos/common/mutex.h"
#include "frc971/control_loops/drivetrain/polydrivetrain_dog_motor_plant.h"
#include "frc971/control_loops/drivetrain/drivetrain_dog_motor_plant.h"
@@ -191,7 +198,20 @@
}
const Values &GetValuesForTeam(uint16_t team_number) {
- return *(DoGetValuesForTeam(team_number));
+ static ::aos::Mutex mutex;
+ ::aos::MutexLocker locker(&mutex);
+
+ // IMPORTANT: This declaration has to stay after the mutex is locked to avoid
+ // race conditions.
+ static ::std::map<uint16_t, const Values *> values;
+
+ if (values.count(team_number) == 0) {
+ values[team_number] = DoGetValuesForTeam(team_number);
+#if __has_feature(address_sanitizer)
+ __lsan_ignore_object(values[team_number]);
+#endif
+ }
+ return *values[team_number];
}
} // namespace constants
diff --git a/frc971/constants.h b/frc971/constants.h
index 67c6a16..173efad 100755
--- a/frc971/constants.h
+++ b/frc971/constants.h
@@ -117,8 +117,12 @@
double drivetrain_max_speed;
};
-// Creates (once) a Values instance and returns a reference to it.
+// Creates (once) a Values instance for ::aos::network::GetTeamNumber() and
+// returns a reference to it.
const Values &GetValues();
+
+// Creates Values instances for each team number it is called with and returns
+// them.
const Values &GetValuesForTeam(uint16_t team_number);
} // namespace constants
diff --git a/frc971/control_loops/claw/claw.cc b/frc971/control_loops/claw/claw.cc
index 22cbf5a..44ec4e8 100644
--- a/frc971/control_loops/claw/claw.cc
+++ b/frc971/control_loops/claw/claw.cc
@@ -51,8 +51,8 @@
static const double kMaxVoltage = 12.0;
const double kRezeroThreshold = 0.03;
-ClawLimitedLoop::ClawLimitedLoop(StateFeedbackLoop<4, 2, 2> loop)
- : StateFeedbackLoop<4, 2, 2>(loop),
+ClawLimitedLoop::ClawLimitedLoop(StateFeedbackLoop<4, 2, 2> &&loop)
+ : StateFeedbackLoop<4, 2, 2>(::std::move(loop)),
uncapped_average_voltage_(0.0),
is_zeroing_(true),
U_Poly_((Eigen::Matrix<double, 4, 2>() << 1, 0,
diff --git a/frc971/control_loops/claw/claw.h b/frc971/control_loops/claw/claw.h
index 13d911e..12f7264 100644
--- a/frc971/control_loops/claw/claw.h
+++ b/frc971/control_loops/claw/claw.h
@@ -25,7 +25,7 @@
class ClawLimitedLoop : public StateFeedbackLoop<4, 2, 2> {
public:
- ClawLimitedLoop(StateFeedbackLoop<4, 2, 2> loop);
+ ClawLimitedLoop(StateFeedbackLoop<4, 2, 2> &&loop);
virtual void CapU();
void set_is_zeroing(bool is_zeroing) { is_zeroing_ = is_zeroing; }
diff --git a/frc971/control_loops/drivetrain/drivetrain.cc b/frc971/control_loops/drivetrain/drivetrain.cc
index ca9c3c7..77d5781 100644
--- a/frc971/control_loops/drivetrain/drivetrain.cc
+++ b/frc971/control_loops/drivetrain/drivetrain.cc
@@ -28,8 +28,8 @@
public:
class LimitedDrivetrainLoop : public StateFeedbackLoop<4, 2, 2> {
public:
- LimitedDrivetrainLoop(const StateFeedbackLoop<4, 2, 2> &loop)
- : StateFeedbackLoop<4, 2, 2>(loop),
+ LimitedDrivetrainLoop(StateFeedbackLoop<4, 2, 2> &&loop)
+ : StateFeedbackLoop<4, 2, 2>(::std::move(loop)),
U_Poly_((Eigen::Matrix<double, 4, 2>() << 1, 0,
-1, 0,
0, 1,
diff --git a/frc971/control_loops/shooter/shooter.h b/frc971/control_loops/shooter/shooter.h
index 1352c49..82a06b6 100755
--- a/frc971/control_loops/shooter/shooter.h
+++ b/frc971/control_loops/shooter/shooter.h
@@ -26,13 +26,13 @@
// that isn't true.
// This class implements the CapU function correctly given all the extra
-// information that we know about from the wrist motor.
+// information that we know about.
// It does not have any zeroing logic in it, only logic to deal with a delta U
// controller.
class ZeroedStateFeedbackLoop : public StateFeedbackLoop<3, 1, 1> {
public:
- ZeroedStateFeedbackLoop(StateFeedbackLoop<3, 1, 1> loop)
- : StateFeedbackLoop<3, 1, 1>(loop),
+ ZeroedStateFeedbackLoop(StateFeedbackLoop<3, 1, 1> &&loop)
+ : StateFeedbackLoop<3, 1, 1>(::std::move(loop)),
voltage_(0.0),
last_voltage_(0.0),
uncapped_voltage_(0.0),
@@ -42,7 +42,6 @@
const static int kZeroingMaxVoltage = 5;
- // Caps U, but this time respects the state of the wrist as well.
virtual void CapU();
// Returns the accumulated voltage.
diff --git a/frc971/control_loops/shooter/shooter_lib_test.cc b/frc971/control_loops/shooter/shooter_lib_test.cc
index 9a55d55..a37cf6b 100755
--- a/frc971/control_loops/shooter/shooter_lib_test.cc
+++ b/frc971/control_loops/shooter/shooter_lib_test.cc
@@ -259,7 +259,7 @@
}
// pointer to plant
- ::std::unique_ptr<StateFeedbackPlant<2, 1, 1>> shooter_plant_;
+ const ::std::unique_ptr<StateFeedbackPlant<2, 1, 1>> shooter_plant_;
// true latch closed
bool latch_piston_state_;
diff --git a/frc971/control_loops/state_feedback_loop.h b/frc971/control_loops/state_feedback_loop.h
index f289e81..d62bc63 100644
--- a/frc971/control_loops/state_feedback_loop.h
+++ b/frc971/control_loops/state_feedback_loop.h
@@ -9,6 +9,11 @@
#include "Eigen/Dense"
#include "aos/common/logging/logging.h"
+#include "aos/common/macros.h"
+
+// TODO(brians): There are a lot of public member variables in here that should
+// be made private and have (const) accessors instead (and have _s at the end of
+// their names).
template <int number_of_states, int number_of_inputs, int number_of_outputs>
class StateFeedbackPlantCoefficients {
@@ -57,6 +62,30 @@
class StateFeedbackPlant {
public:
EIGEN_MAKE_ALIGNED_OPERATOR_NEW;
+
+ StateFeedbackPlant(
+ const ::std::vector<StateFeedbackPlantCoefficients<
+ number_of_states, number_of_inputs,
+ number_of_outputs> *> &coefficients)
+ : coefficients_(coefficients),
+ plant_index_(0) {
+ Reset();
+ }
+
+ StateFeedbackPlant(StateFeedbackPlant &&other)
+ : plant_index_(other.plant_index_) {
+ ::std::swap(coefficients_, other.coefficients_);
+ X.swap(other.X);
+ Y.swap(other.Y);
+ U.swap(other.U);
+ }
+
+ virtual ~StateFeedbackPlant() {
+ for (auto c : coefficients_) {
+ delete c;
+ }
+ }
+
::std::vector<StateFeedbackPlantCoefficients<
number_of_states, number_of_inputs, number_of_outputs> *> coefficients_;
@@ -112,23 +141,6 @@
Eigen::Matrix<double, number_of_outputs, 1> Y;
Eigen::Matrix<double, number_of_inputs, 1> U;
- StateFeedbackPlant(
- const ::std::vector<StateFeedbackPlantCoefficients<
- number_of_states, number_of_inputs,
- number_of_outputs> *> &coefficients)
- : coefficients_(coefficients),
- plant_index_(0) {
- Reset();
- }
-
- StateFeedbackPlant(StateFeedbackPlant &&other)
- : plant_index_(0) {
- Reset();
- ::std::swap(coefficients_, other.coefficients_);
- }
-
- virtual ~StateFeedbackPlant() {}
-
// Assert that U is within the hardware range.
virtual void CheckU() {
for (int i = 0; i < kNumOutputs; ++i) {
@@ -154,6 +166,8 @@
private:
int plant_index_;
+
+ DISALLOW_COPY_AND_ASSIGN(StateFeedbackPlant);
};
// A Controller is a structure which holds a plant and the K and L matrices.
@@ -183,6 +197,51 @@
public:
EIGEN_MAKE_ALIGNED_OPERATOR_NEW;
+ StateFeedbackLoop(const StateFeedbackController<
+ number_of_states, number_of_inputs, number_of_outputs> &controller)
+ : controller_index_(0) {
+ controllers_.push_back(new StateFeedbackController<
+ number_of_states, number_of_inputs, number_of_outputs>(controller));
+ Reset();
+ }
+
+ StateFeedbackLoop(const ::std::vector<StateFeedbackController<
+ number_of_states, number_of_inputs, number_of_outputs> *> &controllers)
+ : controllers_(controllers), controller_index_(0) {
+ Reset();
+ }
+
+ StateFeedbackLoop(StateFeedbackLoop &&other) {
+ X_hat.swap(other.X_hat);
+ R.swap(other.R);
+ U.swap(other.U);
+ U_uncapped.swap(other.U_uncapped);
+ U_ff.swap(other.U_ff);
+ ::std::swap(controllers_, other.controllers_);
+ Y_.swap(other.Y_);
+ new_y_ = other.new_y_;
+ controller_index_ = other.controller_index_;
+ }
+
+ StateFeedbackLoop(
+ const Eigen::Matrix<double, number_of_states, number_of_outputs> &L,
+ const Eigen::Matrix<double, number_of_outputs, number_of_states> &K,
+ const StateFeedbackPlantCoefficients<number_of_states, number_of_inputs,
+ number_of_outputs> &plant)
+ : controller_index_(0) {
+ controllers_.push_back(
+ new StateFeedbackController<number_of_states, number_of_inputs,
+ number_of_outputs>(L, K, plant));
+
+ Reset();
+ }
+
+ virtual ~StateFeedbackLoop() {
+ for (auto c : controllers_) {
+ delete c;
+ }
+ }
+
const Eigen::Matrix<double, number_of_states, number_of_states> &A() const {
return controller().plant.A;
}
@@ -216,12 +275,6 @@
}
double U_max(int i, int j) const { return U_max()(i, j); }
- Eigen::Matrix<double, number_of_states, 1> X_hat;
- Eigen::Matrix<double, number_of_states, 1> R;
- Eigen::Matrix<double, number_of_inputs, 1> U;
- Eigen::Matrix<double, number_of_inputs, 1> U_uncapped;
- Eigen::Matrix<double, number_of_outputs, 1> U_ff;
-
const StateFeedbackController<number_of_states, number_of_inputs,
number_of_outputs> &controller() const {
return *controllers_[controller_index_];
@@ -241,34 +294,6 @@
U_ff.setZero();
}
- StateFeedbackLoop(const StateFeedbackController<
- number_of_states, number_of_inputs, number_of_outputs> &controller)
- : controller_index_(0) {
- controllers_.push_back(new StateFeedbackController<
- number_of_states, number_of_inputs, number_of_outputs>(controller));
- Reset();
- }
-
- StateFeedbackLoop(const ::std::vector<StateFeedbackController<
- number_of_states, number_of_inputs, number_of_outputs> *> &controllers)
- : controllers_(controllers), controller_index_(0) {
- Reset();
- }
-
- StateFeedbackLoop(
- const Eigen::Matrix<double, number_of_states, number_of_outputs> &L,
- const Eigen::Matrix<double, number_of_outputs, number_of_states> &K,
- const StateFeedbackPlantCoefficients<number_of_states, number_of_inputs,
- number_of_outputs> &plant)
- : controller_index_(0) {
- controllers_.push_back(
- new StateFeedbackController<number_of_states, number_of_inputs,
- number_of_outputs>(L, K, plant));
-
- Reset();
- }
- virtual ~StateFeedbackLoop() {}
-
virtual void FeedForward() {
for (int i = 0; i < number_of_outputs; ++i) {
U_ff[i] = 0.0;
@@ -350,6 +375,12 @@
int controller_index() const { return controller_index_; }
+ Eigen::Matrix<double, number_of_states, 1> X_hat;
+ Eigen::Matrix<double, number_of_states, 1> R;
+ Eigen::Matrix<double, number_of_inputs, 1> U;
+ Eigen::Matrix<double, number_of_inputs, 1> U_uncapped;
+ Eigen::Matrix<double, number_of_outputs, 1> U_ff;
+
protected:
::std::vector<StateFeedbackController<number_of_states, number_of_inputs,
number_of_outputs> *> controllers_;
@@ -365,6 +396,9 @@
bool new_y_ = false;
int controller_index_;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(StateFeedbackLoop);
};
#endif // FRC971_CONTROL_LOOPS_STATEFEEDBACKLOOP_H_