diff --git a/hal/include/HAL/Notifier.h b/hal/include/HAL/Notifier.h index 4c07f69970..27c69f013d 100644 --- a/hal/include/HAL/Notifier.h +++ b/hal/include/HAL/Notifier.h @@ -14,7 +14,7 @@ #ifdef __cplusplus extern "C" { #endif -HAL_NotifierHandle HAL_InitializeNotifier(void (*process)(uint64_t, void*), +HAL_NotifierHandle HAL_InitializeNotifier(void (*process)(uint64_t, HAL_NotifierHandle), void* param, int32_t* status); void HAL_CleanNotifier(HAL_NotifierHandle notifierHandle, int32_t* status); void* HAL_GetNotifierParam(HAL_NotifierHandle notifierHandle, int32_t* status); diff --git a/hal/lib/athena/HALAthena.cpp b/hal/lib/athena/HALAthena.cpp index 5221eb5903..912b3685fe 100644 --- a/hal/lib/athena/HALAthena.cpp +++ b/hal/lib/athena/HALAthena.cpp @@ -250,10 +250,10 @@ static void HALCleanupAtExit() { setNewDataSem(nullptr); } -static void timerRollover(uint64_t currentTime, void*) { +static void timerRollover(uint64_t currentTime, HAL_NotifierHandle handle) { // reschedule timer for next rollover int32_t status = 0; - HAL_UpdateNotifierAlarm(rolloverNotifier, currentTime + 0x80000000ULL, + HAL_UpdateNotifierAlarm(handle, currentTime + 0x80000000ULL, &status); } diff --git a/hal/lib/athena/Notifier.cpp b/hal/lib/athena/Notifier.cpp index 7d1439d772..6b64f6ab33 100644 --- a/hal/lib/athena/Notifier.cpp +++ b/hal/lib/athena/Notifier.cpp @@ -32,8 +32,9 @@ namespace { struct Notifier { std::shared_ptr prev, next; void* param; - void (*process)(uint64_t, void*); + void (*process)(uint64_t, HAL_NotifierHandle); uint64_t triggerTime = UINT64_MAX; + HAL_NotifierHandle handle; }; } static std::shared_ptr notifiers; @@ -90,9 +91,9 @@ static void alarmCallback(uint32_t, void*) { if (notifier->triggerTime < currentTime) { notifier->triggerTime = UINT64_MAX; auto process = notifier->process; - auto param = notifier->param; + auto handle = notifier->handle; sync.unlock(); - process(currentTime, param); + process(currentTime, handle); sync.lock(); } else if (notifier->triggerTime < closestTrigger) { updateNotifierAlarmInternal(notifier, notifier->triggerTime, &status); @@ -109,7 +110,7 @@ static void cleanupNotifierAtExit() { extern "C" { -HAL_NotifierHandle HAL_InitializeNotifier(void (*process)(uint64_t, void*), +HAL_NotifierHandle HAL_InitializeNotifier(void (*process)(uint64_t, HAL_NotifierHandle), void* param, int32_t* status) { if (!process) { *status = NULL_PARAMETER; @@ -130,14 +131,20 @@ HAL_NotifierHandle HAL_InitializeNotifier(void (*process)(uint64_t, void*), } std::lock_guard sync(notifierMutex); - // create notifier structure and add to list std::shared_ptr notifier = std::make_shared(); + HAL_NotifierHandle handle = notifierHandles.Allocate(notifier); + if (handle == HAL_kInvalidHandle) { + *status = HAL_HANDLE_ERROR; + return HAL_kInvalidHandle; + } + // create notifier structure and add to list notifier->next = notifiers; if (notifier->next) notifier->next->prev = notifier; notifier->param = param; notifier->process = process; + notifier->handle = handle; notifiers = notifier; - return notifierHandles.Allocate(notifier); + return handle; } void HAL_CleanNotifier(HAL_NotifierHandle notifierHandle, int32_t* status) { diff --git a/hal/lib/athena/SPI.cpp b/hal/lib/athena/SPI.cpp index a58e788afa..7a1ed96ba9 100644 --- a/hal/lib/athena/SPI.cpp +++ b/hal/lib/athena/SPI.cpp @@ -328,7 +328,10 @@ void HAL_SetSPIHandle(int32_t port, int32_t handle) { } } -static void spiAccumulatorProcess(uint64_t currentTime, void* param) { +static void spiAccumulatorProcess(uint64_t currentTime, HAL_NotifierHandle handle) { + int32_t status = 0; + auto param = HAL_GetNotifierParam(handle, &status); + if (param == nullptr) return; SPIAccumulator* accum = static_cast(param); // perform SPI transaction @@ -376,7 +379,7 @@ static void spiAccumulatorProcess(uint64_t currentTime, void* param) { // handle timer slip if (accum->triggerTime < currentTime) accum->triggerTime = currentTime + accum->period; - int32_t status = 0; + status = 0; HAL_UpdateNotifierAlarm(accum->notifier, accum->triggerTime, &status); } diff --git a/wpilibc/athena/include/Notifier.h b/wpilibc/athena/include/Notifier.h index f4b7462c99..24ea549b18 100644 --- a/wpilibc/athena/include/Notifier.h +++ b/wpilibc/athena/include/Notifier.h @@ -38,8 +38,10 @@ class Notifier : public ErrorBase { // update the HAL alarm void UpdateAlarm(); // HAL callback - static void Notify(uint64_t currentTimeInt, void* param); + static void Notify(uint64_t currentTimeInt, HAL_NotifierHandle handle); + // used to constrain execution between destructors and callback + static priority_mutex m_destructorMutex; // held while updating process information priority_mutex m_processMutex; // HAL handle, atomic for proper destruction @@ -52,7 +54,4 @@ class Notifier : public ErrorBase { double m_period = 0; // true if this is a periodic event bool m_periodic = false; - - // held HAL notifier while Notifier::Notify() call is in progress - priority_mutex m_notifyMutex; }; diff --git a/wpilibc/athena/src/Notifier.cpp b/wpilibc/athena/src/Notifier.cpp index 8c7175c584..b3a67b8ced 100644 --- a/wpilibc/athena/src/Notifier.cpp +++ b/wpilibc/athena/src/Notifier.cpp @@ -11,6 +11,8 @@ #include "Utility.h" #include "WPIErrors.h" +priority_mutex Notifier::m_destructorMutex; + /** * Create a Notifier for timer event notification. * @@ -39,7 +41,8 @@ Notifier::~Notifier() { /* Acquire the mutex; this makes certain that the handler is not being * executed by the interrupt manager. */ - std::lock_guard lock(m_notifyMutex); + std::lock_guard lockStatic(Notifier::m_destructorMutex); + std::lock_guard lock(m_processMutex); } /** @@ -47,6 +50,8 @@ Notifier::~Notifier() { */ void Notifier::UpdateAlarm() { int32_t status = 0; + // Return if we are being destructed, or were not created successfully + if (m_notifier == 0) return; HAL_UpdateNotifierAlarm( m_notifier, static_cast(m_expirationTime * 1e6), &status); wpi_setErrorWithContext(status, HAL_GetErrorMessage(status)); @@ -56,10 +61,18 @@ void Notifier::UpdateAlarm() { * Notify is called by the HAL layer. We simply need to pass it through to * the user handler. */ -void Notifier::Notify(uint64_t currentTimeInt, void* param) { - Notifier* notifier = static_cast(param); +void Notifier::Notify(uint64_t currentTimeInt, HAL_NotifierHandle handle) { + Notifier* notifier; + { + // Lock static mutex to grab the notifier param + std::lock_guard lock(Notifier::m_destructorMutex); + int32_t status = 0; + auto notifierPointer = HAL_GetNotifierParam(handle, &status); + if (notifierPointer == nullptr) return; + notifier = static_cast(notifierPointer); + notifier->m_processMutex.lock(); + } - notifier->m_processMutex.lock(); if (notifier->m_periodic) { notifier->m_expirationTime += notifier->m_period; notifier->UpdateAlarm(); @@ -67,11 +80,8 @@ void Notifier::Notify(uint64_t currentTimeInt, void* param) { auto handler = notifier->m_handler; - notifier->m_notifyMutex.lock(); - notifier->m_processMutex.unlock(); - if (handler) handler(); - notifier->m_notifyMutex.unlock(); + notifier->m_processMutex.unlock(); } /** @@ -123,5 +133,6 @@ void Notifier::Stop() { // Wait for a currently executing handler to complete before returning from // Stop() - std::lock_guard sync(m_notifyMutex); + std::lock_guard lockStatic(Notifier::m_destructorMutex); + std::lock_guard lock(m_processMutex); } diff --git a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp index 157439fb9f..420d02f7f2 100644 --- a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp +++ b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp @@ -106,8 +106,11 @@ void NotifierThreadJNI::Main() { jvm->DetachCurrentThread(); } -void notifierHandler(uint64_t currentTimeInt, void *param) { - ((NotifierJNI *)param)->Notify(currentTimeInt); +void notifierHandler(uint64_t currentTimeInt, HAL_NotifierHandle handle) { + int32_t status = 0; + auto param = HAL_GetNotifierParam(handle, &status); + if (param == nullptr) return; + (static_cast(param))->Notify(currentTimeInt); } extern "C" {