From f67849a9b171d12211694b3672fd7e50ca18c33d Mon Sep 17 00:00:00 2001 From: James Kuszmaul Date: Sat, 8 Nov 2014 14:27:49 -0500 Subject: [PATCH] [artf3749] Repaired undefined behavior in takeMultiWait. Change-Id: Ieae7d602472f585db2a896cc76355a5c23d1d670 --- hal/include/HAL/Semaphore.hpp | 2 +- hal/lib/Athena/Semaphore.cpp | 6 ++---- wpilibc/wpilibC++Devices/include/DriverStation.h | 1 + wpilibc/wpilibC++Devices/src/DriverStation.cpp | 4 +++- wpilibc/wpilibC++Sim/include/DriverStation.h | 5 +++-- wpilibc/wpilibC++Sim/include/simulation/simTime.h | 1 + wpilibc/wpilibC++Sim/src/DriverStation.cpp | 4 +++- wpilibc/wpilibC++Sim/src/Timer.cpp | 4 +++- 8 files changed, 17 insertions(+), 10 deletions(-) diff --git a/hal/include/HAL/Semaphore.hpp b/hal/include/HAL/Semaphore.hpp index bc3e268354..8b81c9526c 100644 --- a/hal/include/HAL/Semaphore.hpp +++ b/hal/include/HAL/Semaphore.hpp @@ -46,7 +46,7 @@ extern "C" MULTIWAIT_ID initializeMultiWait(); void deleteMultiWait(MULTIWAIT_ID sem); - int8_t takeMultiWait(MULTIWAIT_ID sem, int32_t timeout); + int8_t takeMultiWait(MULTIWAIT_ID sem, MUTEX_ID m, int32_t timeout); int8_t giveMultiWait(MULTIWAIT_ID sem); } diff --git a/hal/lib/Athena/Semaphore.cpp b/hal/lib/Athena/Semaphore.cpp index e76b8ad139..0ab17669b6 100644 --- a/hal/lib/Athena/Semaphore.cpp +++ b/hal/lib/Athena/Semaphore.cpp @@ -1,4 +1,3 @@ - #include "HAL/Semaphore.hpp" #include "Log.hpp" @@ -122,11 +121,10 @@ void deleteMultiWait(MULTIWAIT_ID sem) { delete sem; } -int8_t takeMultiWait(MULTIWAIT_ID sem, int32_t timeout) { - MUTEX_ID m = initializeMutexNormal(); +int8_t takeMultiWait(MULTIWAIT_ID sem, MUTEX_ID m, int32_t timeout) { takeMutex(m); int8_t val = pthread_cond_wait(sem, m); - deleteMutex(m); + giveMutex(m); return val; } diff --git a/wpilibc/wpilibC++Devices/include/DriverStation.h b/wpilibc/wpilibC++Devices/include/DriverStation.h index 996024a3ba..46b9e680c8 100644 --- a/wpilibc/wpilibC++Devices/include/DriverStation.h +++ b/wpilibc/wpilibC++Devices/include/DriverStation.h @@ -107,6 +107,7 @@ private: SEMAPHORE_ID m_newControlData; MUTEX_ID m_packetDataAvailableSem; MULTIWAIT_ID m_waitForDataSem; + MUTEX_ID m_waitForDataMutex; double m_approxMatchTimeOffset; bool m_userInDisabled; bool m_userInAutonomous; diff --git a/wpilibc/wpilibC++Devices/src/DriverStation.cpp b/wpilibc/wpilibC++Devices/src/DriverStation.cpp index 7a77f115e7..fd888e8a10 100644 --- a/wpilibc/wpilibC++Devices/src/DriverStation.cpp +++ b/wpilibc/wpilibC++Devices/src/DriverStation.cpp @@ -61,6 +61,7 @@ DriverStation::DriverStation() HALSetNewDataSem(m_packetDataAvailableSem); m_waitForDataSem = initializeMultiWait(); + m_waitForDataMutex = initializeMutexNormal(); AddToSingletonList(); @@ -79,6 +80,7 @@ DriverStation::~DriverStation() // Unregister our semaphore. HALSetNewDataSem(0); deleteMutex(m_packetDataAvailableSem); + deleteMutex(m_waitForDataMutex); } void DriverStation::InitTask(DriverStation *ds) @@ -353,7 +355,7 @@ uint32_t DriverStation::GetLocation() */ void DriverStation::WaitForData() { - takeMultiWait(m_waitForDataSem, SEMAPHORE_WAIT_FOREVER); + takeMultiWait(m_waitForDataSem, m_waitForDataMutex, SEMAPHORE_WAIT_FOREVER); } /** diff --git a/wpilibc/wpilibC++Sim/include/DriverStation.h b/wpilibc/wpilibC++Sim/include/DriverStation.h index 9816f025dc..a337cad1e4 100644 --- a/wpilibc/wpilibC++Sim/include/DriverStation.h +++ b/wpilibc/wpilibC++Sim/include/DriverStation.h @@ -118,8 +118,9 @@ private: uint8_t m_digitalOut; MULTIWAIT_ID m_waitForDataSem; - MUTEX_ID m_stateSemaphore; - MUTEX_ID m_joystickSemaphore; + MUTEX_ID m_waitForDataMutex; + MUTEX_ID m_stateSemaphore; + MUTEX_ID m_joystickSemaphore; double m_approxMatchTimeOffset; bool m_userInDisabled; bool m_userInAutonomous; diff --git a/wpilibc/wpilibC++Sim/include/simulation/simTime.h b/wpilibc/wpilibC++Sim/include/simulation/simTime.h index 7b4bf90690..aada938ea0 100644 --- a/wpilibc/wpilibC++Sim/include/simulation/simTime.h +++ b/wpilibc/wpilibC++Sim/include/simulation/simTime.h @@ -3,5 +3,6 @@ namespace wpilib { namespace internal { extern double simTime; extern MULTIWAIT_ID time_wait; + extern MUTEX_ID time_wait_mutex; // transport::SubscriberPtr time_sub; }} diff --git a/wpilibc/wpilibC++Sim/src/DriverStation.cpp b/wpilibc/wpilibC++Sim/src/DriverStation.cpp index f78fa1119c..192c0ebfa4 100644 --- a/wpilibc/wpilibC++Sim/src/DriverStation.cpp +++ b/wpilibc/wpilibC++Sim/src/DriverStation.cpp @@ -45,6 +45,7 @@ DriverStation::DriverStation() { // Create a new semaphore m_waitForDataSem = initializeMultiWait(); + m_waitForDataMutex = initializeMutexNormal(); m_stateSemaphore = initializeMutexRecursive(); m_joystickSemaphore = initializeMutexRecursive(); @@ -78,6 +79,7 @@ DriverStation::~DriverStation() { m_instance = NULL; deleteMultiWait(m_waitForDataSem); + deleteMutex(m_waitForDataMutex); // TODO: Release m_stateSemaphore and m_joystickSemaphore? } @@ -311,7 +313,7 @@ uint32_t DriverStation::GetLocation() */ void DriverStation::WaitForData() { - takeMultiWait(m_waitForDataSem, SEMAPHORE_WAIT_FOREVER); + takeMultiWait(m_waitForDataSem, m_waitForDataMutex, SEMAPHORE_WAIT_FOREVER); } /** diff --git a/wpilibc/wpilibC++Sim/src/Timer.cpp b/wpilibc/wpilibC++Sim/src/Timer.cpp index dacb00ecc5..cbf25cdfc8 100644 --- a/wpilibc/wpilibC++Sim/src/Timer.cpp +++ b/wpilibc/wpilibC++Sim/src/Timer.cpp @@ -28,7 +28,8 @@ void Wait(double seconds) double start = wpilib::internal::simTime; while ((wpilib::internal::simTime - start) < seconds) { - takeMultiWait(wpilib::internal::time_wait, 0); + takeMultiWait(wpilib::internal::time_wait, + wpilib::internal::time_wait_mutex, 0); } } @@ -203,6 +204,7 @@ extern "C" namespace wpilib { namespace internal { double simTime = 0; MULTIWAIT_ID time_wait = initializeMultiWait(); + MUTEX_ID time_wait_mutex = initializeMutexNormal(); void time_callback(const msgs::ConstFloat64Ptr &msg) { simTime = msg->data();