From 776a991d615a74d6d09a047f41916fc1ae3c9c9f Mon Sep 17 00:00:00 2001 From: Thad House Date: Sun, 5 Jun 2016 07:29:47 -0700 Subject: [PATCH] Moves Notifier over to handles (#97) Internally, the linked list now uses shared_ptrs instead of raw pointers. In addition, in the WPILib the notifier handle is now made atomic. Then before the class is destructed, the handle is now set to 0. This should help solve one of the existing race conditions. A 0 handle is correctly handled down at the HAL level. --- hal/include/HAL/Handles.h | 2 - hal/include/HAL/Notifier.h | 18 +- hal/lib/athena/HALAthena.cpp | 2 +- hal/lib/athena/Notifier.cpp | 259 +++++++++--------- hal/lib/athena/SPI.cpp | 7 +- hal/lib/athena/handles/HandlesInternal.cpp | 5 +- hal/lib/athena/handles/HandlesInternal.h | 3 + .../athena/handles/UnlimitedHandleResource.h | 9 +- wpilibc/athena/include/Notifier.h | 6 +- wpilibc/athena/src/Notifier.cpp | 4 +- wpilibj/src/athena/cpp/lib/NotifierJNI.cpp | 38 +-- .../java/edu/wpi/first/wpilibj/Notifier.java | 12 +- .../wpi/first/wpilibj/hal/NotifierJNI.java | 8 +- 13 files changed, 197 insertions(+), 176 deletions(-) diff --git a/hal/include/HAL/Handles.h b/hal/include/HAL/Handles.h index ad8722f3a8..2110b5c22b 100644 --- a/hal/include/HAL/Handles.h +++ b/hal/include/HAL/Handles.h @@ -17,5 +17,3 @@ #define HAL_HANDLE_NEGATIVE_INDEX -5 typedef int32_t HalHandle; - -enum HalHandleEnum { Undefined = 0, DIO = 1 }; diff --git a/hal/include/HAL/Notifier.h b/hal/include/HAL/Notifier.h index 84c4328e13..74f0686f29 100644 --- a/hal/include/HAL/Notifier.h +++ b/hal/include/HAL/Notifier.h @@ -9,12 +9,16 @@ #include +#include "Handles.h" + +typedef HalHandle HalNotifierHandle; + extern "C" { -void* initializeNotifier(void (*process)(uint64_t, void*), void* param, - int32_t* status); -void cleanNotifier(void* notifier_pointer, int32_t* status); -void* getNotifierParam(void* notifier_pointer, int32_t* status); -void updateNotifierAlarm(void* notifier_pointer, uint64_t triggerTime, - int32_t* status); -void stopNotifierAlarm(void* notifier_pointer, int32_t* status); +HalNotifierHandle initializeNotifier(void (*process)(uint64_t, void*), + void* param, int32_t* status); +void cleanNotifier(HalNotifierHandle notifier_handle, int32_t* status); +void* getNotifierParam(HalNotifierHandle notifier_handle, int32_t* status); +void updateNotifierAlarm(HalNotifierHandle notifier_handle, + uint64_t triggerTime, int32_t* status); +void stopNotifierAlarm(HalNotifierHandle notifier_handle, int32_t* status); } diff --git a/hal/lib/athena/HALAthena.cpp b/hal/lib/athena/HALAthena.cpp index 504ae937e9..eda8af4190 100644 --- a/hal/lib/athena/HALAthena.cpp +++ b/hal/lib/athena/HALAthena.cpp @@ -40,7 +40,7 @@ static priority_mutex timeMutex; static priority_mutex msgMutex; static uint32_t timeEpoch = 0; static uint32_t prevFPGATime = 0; -static void* rolloverNotifier = nullptr; +static HalNotifierHandle rolloverNotifier = 0; extern "C" { diff --git a/hal/lib/athena/Notifier.cpp b/hal/lib/athena/Notifier.cpp index bd81cf0634..7863c5c077 100644 --- a/hal/lib/athena/Notifier.cpp +++ b/hal/lib/athena/Notifier.cpp @@ -10,11 +10,13 @@ #include #include +#include #include #include "ChipObject.h" #include "HAL/HAL.h" #include "HAL/cpp/priority_mutex.h" +#include "handles/UnlimitedHandleResource.h" static const uint32_t kTimerInterruptNumber = 28; @@ -23,143 +25,33 @@ static priority_recursive_mutex notifierMutex; static tAlarm* notifierAlarm = nullptr; static tInterruptManager* notifierManager = nullptr; static uint64_t closestTrigger = UINT64_MAX; + +namespace { struct Notifier { - Notifier *prev, *next, *freeNext; - bool freed; + std::shared_ptr prev, next; void* param; void (*process)(uint64_t, void*); uint64_t triggerTime = UINT64_MAX; }; -static Notifier* notifiers = nullptr; -static Notifier* notifiersFreeList = nullptr; +} +static std::shared_ptr notifiers = nullptr; static std::atomic_flag notifierAtexitRegistered = ATOMIC_FLAG_INIT; static std::atomic_int notifierRefCount{0}; -static void alarmCallback(uint32_t, void*) { - std::unique_lock sync(notifierMutex); +using namespace hal; - int32_t status = 0; - uint64_t currentTime = 0; +static UnlimitedHandleResource + notifierHandles; - // the hardware disables itself after each alarm - closestTrigger = UINT64_MAX; - - // process all notifiers - Notifier* notifier = notifiers; - while (notifier) { - if (!notifier->freed && notifier->triggerTime != UINT64_MAX) { - if (currentTime == 0) currentTime = getFPGATime(&status); - if (notifier->triggerTime < currentTime) { - notifier->triggerTime = UINT64_MAX; - auto process = notifier->process; - auto param = notifier->param; - sync.unlock(); - process(currentTime, param); - sync.lock(); - } else if (notifier->triggerTime < closestTrigger) { - updateNotifierAlarm(notifier, notifier->triggerTime, &status); - } - } - notifier = notifier->next; - } - - // clean up freelist - notifier = notifiersFreeList; - while (notifier) { - Notifier* next = notifier->freeNext; - delete notifier; - notifier = next; - } - notifiersFreeList = nullptr; -} - -static void cleanupNotifierAtExit() { - notifierAlarm = nullptr; - notifierManager = nullptr; -} - -extern "C" { - -void* initializeNotifier(void (*process)(uint64_t, void*), void* param, - int32_t* status) { - if (!process) { - *status = NULL_PARAMETER; - return nullptr; - } - if (!notifierAtexitRegistered.test_and_set()) - std::atexit(cleanupNotifierAtExit); - if (notifierRefCount.fetch_add(1) == 0) { - std::lock_guard sync(notifierInterruptMutex); - // create manager and alarm if not already created - if (!notifierManager) { - notifierManager = - new tInterruptManager(1 << kTimerInterruptNumber, false, status); - notifierManager->registerHandler(alarmCallback, nullptr, status); - notifierManager->enable(status); - } - if (!notifierAlarm) notifierAlarm = tAlarm::create(status); - } - - std::lock_guard sync(notifierMutex); - // create notifier structure and add to list - Notifier* notifier = new Notifier(); - notifier->prev = nullptr; - notifier->next = notifiers; - notifier->freeNext = nullptr; - if (notifier->next) notifier->next->prev = notifier; - notifier->freed = false; - notifier->param = param; - notifier->process = process; - notifiers = notifier; - return notifier; -} - -void cleanNotifier(void* notifier_pointer, int32_t* status) { - { - std::lock_guard sync(notifierMutex); - Notifier* notifier = (Notifier*)notifier_pointer; - - // ignore double-free - if (notifier->freed) return; - notifier->freed = true; - - // remove from list - if (notifier->prev) notifier->prev->next = notifier->next; - if (notifier->next) notifier->next->prev = notifier->prev; - if (notifiers == notifier) notifiers = notifier->next; - - // add to freelist - notifier->freeNext = notifiersFreeList; - notifiersFreeList = notifier; - } - - if (notifierRefCount.fetch_sub(1) == 1) { - std::lock_guard sync(notifierInterruptMutex); - // if this was the last notifier, clean up alarm and manager - if (notifierAlarm) { - notifierAlarm->writeEnable(false, status); - delete notifierAlarm; - notifierAlarm = nullptr; - } - if (notifierManager) { - notifierManager->disable(status); - delete notifierManager; - notifierManager = nullptr; - } - closestTrigger = UINT64_MAX; - } -} - -void* getNotifierParam(void* notifier_pointer, int32_t* status) { - return ((Notifier*)notifier_pointer)->param; -} - -void updateNotifierAlarm(void* notifier_pointer, uint64_t triggerTime, - int32_t* status) { +// internal version of updateAlarm used during the alarmCallback when we know +// that the pointer is a valid pointer. +void updateNotifierAlarmInternal(std::shared_ptr notifier_pointer, + uint64_t triggerTime, int32_t* status) { std::lock_guard sync(notifierMutex); - Notifier* notifier = (Notifier*)notifier_pointer; - if (notifier->freed) return; + auto notifier = notifier_pointer; + // no need for a null check, as this must always be a valid pointer. notifier->triggerTime = triggerTime; bool wasActive = (closestTrigger != UINT64_MAX); @@ -179,9 +71,122 @@ void updateNotifierAlarm(void* notifier_pointer, uint64_t triggerTime, notifierInterruptMutex.unlock(); } -void stopNotifierAlarm(void* notifier_pointer, int32_t* status) { +static void alarmCallback(uint32_t, void*) { + std::unique_lock sync(notifierMutex); + + int32_t status = 0; + uint64_t currentTime = 0; + + // the hardware disables itself after each alarm + closestTrigger = UINT64_MAX; + + // process all notifiers + std::shared_ptr notifier = notifiers; + while (notifier) { + if (notifier->triggerTime != UINT64_MAX) { + if (currentTime == 0) currentTime = getFPGATime(&status); + if (notifier->triggerTime < currentTime) { + notifier->triggerTime = UINT64_MAX; + auto process = notifier->process; + auto param = notifier->param; + sync.unlock(); + process(currentTime, param); + sync.lock(); + } else if (notifier->triggerTime < closestTrigger) { + updateNotifierAlarmInternal(notifier, notifier->triggerTime, &status); + } + } + notifier = notifier->next; + } +} + +static void cleanupNotifierAtExit() { + notifierAlarm = nullptr; + notifierManager = nullptr; +} + +extern "C" { + +HalNotifierHandle initializeNotifier(void (*process)(uint64_t, void*), + void* param, int32_t* status) { + if (!process) { + *status = NULL_PARAMETER; + return 0; + } + if (!notifierAtexitRegistered.test_and_set()) + std::atexit(cleanupNotifierAtExit); + if (notifierRefCount.fetch_add(1) == 0) { + std::lock_guard sync(notifierInterruptMutex); + // create manager and alarm if not already created + if (!notifierManager) { + notifierManager = + new tInterruptManager(1 << kTimerInterruptNumber, false, status); + notifierManager->registerHandler(alarmCallback, nullptr, status); + notifierManager->enable(status); + } + if (!notifierAlarm) notifierAlarm = tAlarm::create(status); + } + std::lock_guard sync(notifierMutex); - Notifier* notifier = (Notifier*)notifier_pointer; + // create notifier structure and add to list + std::shared_ptr notifier = std::make_shared(); + notifier->next = notifiers; + if (notifier->next) notifier->next->prev = notifier; + notifier->param = param; + notifier->process = process; + notifiers = notifier; + return notifierHandles.Allocate(notifier); +} + +void cleanNotifier(HalNotifierHandle notifier_handle, int32_t* status) { + { + std::lock_guard sync(notifierMutex); + auto notifier = notifierHandles.Get(notifier_handle); + if (!notifier) return; + + // remove from list + if (notifier->prev) notifier->prev->next = notifier->next; + if (notifier->next) notifier->next->prev = notifier->prev; + if (notifiers == notifier) notifiers = notifier->next; + notifierHandles.Free(notifier_handle); + } + + if (notifierRefCount.fetch_sub(1) == 1) { + std::lock_guard sync(notifierInterruptMutex); + // if this was the last notifier, clean up alarm and manager + if (notifierAlarm) { + notifierAlarm->writeEnable(false, status); + delete notifierAlarm; + notifierAlarm = nullptr; + } + if (notifierManager) { + notifierManager->disable(status); + delete notifierManager; + notifierManager = nullptr; + } + closestTrigger = UINT64_MAX; + } +} + +void* getNotifierParam(HalNotifierHandle notifier_handle, int32_t* status) { + auto notifier = notifierHandles.Get(notifier_handle); + if (!notifier) return nullptr; + return notifier->param; +} + +void updateNotifierAlarm(HalNotifierHandle notifier_handle, + uint64_t triggerTime, int32_t* status) { + std::lock_guard sync(notifierMutex); + + auto notifier = notifierHandles.Get(notifier_handle); + if (!notifier) return; + updateNotifierAlarmInternal(notifier, triggerTime, status); +} + +void stopNotifierAlarm(HalNotifierHandle notifier_handle, int32_t* status) { + std::lock_guard sync(notifierMutex); + auto notifier = notifierHandles.Get(notifier_handle); + if (!notifier) return; notifier->triggerTime = UINT64_MAX; } diff --git a/hal/lib/athena/SPI.cpp b/hal/lib/athena/SPI.cpp index be90346350..4866eda12f 100644 --- a/hal/lib/athena/SPI.cpp +++ b/hal/lib/athena/SPI.cpp @@ -7,6 +7,8 @@ #include "HAL/SPI.h" +#include + #include "DigitalInternal.h" #include "HAL/HAL.h" #include "spilib/spi-lib.h" @@ -28,7 +30,7 @@ static tSPI* spiSystem; extern "C" { struct SPIAccumulator { - void* notifier = nullptr; + std::atomic notifier{0}; uint64_t triggerTime; uint32_t period; @@ -413,7 +415,8 @@ void spiFreeAccumulator(uint8_t port, int32_t* status) { *status = NULL_PARAMETER; return; } - cleanNotifier(accum->notifier, status); + HalNotifierHandle handle = accum->notifier.exchange(0); + cleanNotifier(handle, status); delete accum; spiAccumulators[port] = nullptr; } diff --git a/hal/lib/athena/handles/HandlesInternal.cpp b/hal/lib/athena/handles/HandlesInternal.cpp index b1ff98282d..08963cf6b1 100644 --- a/hal/lib/athena/handles/HandlesInternal.cpp +++ b/hal/lib/athena/handles/HandlesInternal.cpp @@ -13,9 +13,10 @@ namespace hal { HalHandle createHandle(int16_t index, HalHandleEnum handleType) { if (index < 0) return HAL_HANDLE_NEGATIVE_INDEX; - if (handleType <= 0 || handleType > 127) return HAL_HANDLE_INVALID_TYPE; + uint8_t hType = static_cast(handleType); + if (hType == 0 || hType > 127) return HAL_HANDLE_INVALID_TYPE; // set last 8 bits, then shift to first 8 bits - HalHandle handle = handleType; + HalHandle handle = hType; handle = handle << 24; // add index to set last 16 bits handle += index; diff --git a/hal/lib/athena/handles/HandlesInternal.h b/hal/lib/athena/handles/HandlesInternal.h index 8717dc85d4..a2633f9f4c 100644 --- a/hal/lib/athena/handles/HandlesInternal.h +++ b/hal/lib/athena/handles/HandlesInternal.h @@ -22,6 +22,9 @@ */ namespace hal { + +enum class HalHandleEnum { Undefined = 0, DIO = 1, Port = 2, Notifier = 3 }; + static inline int16_t getHandleIndex(HalHandle handle) { // mask and return last 16 bits return (int16_t)(handle & 0xffff); diff --git a/hal/lib/athena/handles/UnlimitedHandleResource.h b/hal/lib/athena/handles/UnlimitedHandleResource.h index 5dbc608da3..ca862e379a 100644 --- a/hal/lib/athena/handles/UnlimitedHandleResource.h +++ b/hal/lib/athena/handles/UnlimitedHandleResource.h @@ -53,7 +53,7 @@ template THandle UnlimitedHandleResource::Allocate( std::shared_ptr structure) { std::lock_guard sync(m_handleMutex); - int16_t i; + size_t i; for (i = 0; i < m_structures.size(); i++) { if (m_structures[i] == nullptr) { m_structures[i] = structure; @@ -63,7 +63,7 @@ THandle UnlimitedHandleResource::Allocate( if (i >= INT16_MAX) return HAL_HANDLE_OUT_OF_HANDLES; m_structures.push_back(structure); - return (THandle)createHandle(i, enumValue); + return (THandle)createHandle(static_cast(i), enumValue); } template @@ -71,7 +71,8 @@ std::shared_ptr UnlimitedHandleResource::Get(THandle handle) { int16_t index = getHandleTypedIndex(handle, enumValue); std::lock_guard sync(m_handleMutex); - if (index < 0 || index >= m_structures.size()) return nullptr; + if (index < 0 || index >= static_cast(m_structures.size())) + return nullptr; return m_structures[index]; } @@ -80,7 +81,7 @@ void UnlimitedHandleResource::Free( THandle handle) { int16_t index = getHandleTypedIndex(handle, enumValue); std::lock_guard sync(m_handleMutex); - if (index < 0 || index >= m_structures.size()) return; + if (index < 0 || index >= static_cast(m_structures.size())) return; m_structures[index].reset(); } } diff --git a/wpilibc/athena/include/Notifier.h b/wpilibc/athena/include/Notifier.h index d4f6ac94c2..1f8adff23c 100644 --- a/wpilibc/athena/include/Notifier.h +++ b/wpilibc/athena/include/Notifier.h @@ -7,9 +7,11 @@ #pragma once +#include #include #include "ErrorBase.h" +#include "HAL/Notifier.h" #include "HAL/cpp/priority_mutex.h" typedef std::function TimerEventHandler; @@ -40,8 +42,8 @@ class Notifier : public ErrorBase { // held while updating process information priority_mutex m_processMutex; - // HAL handle - void* m_notifier; + // HAL handle, atomic for proper destruction + std::atomic m_notifier{0}; // address of the handler TimerEventHandler m_handler; // the absolute expiration time diff --git a/wpilibc/athena/src/Notifier.cpp b/wpilibc/athena/src/Notifier.cpp index 81be782f27..ebd384dc27 100644 --- a/wpilibc/athena/src/Notifier.cpp +++ b/wpilibc/athena/src/Notifier.cpp @@ -31,7 +31,9 @@ Notifier::Notifier(TimerEventHandler handler) { */ Notifier::~Notifier() { int32_t status = 0; - cleanNotifier(m_notifier, &status); + // atomically set handle to 0, then clean + HalNotifierHandle handle = m_notifier.exchange(0); + cleanNotifier(handle, &status); wpi_setErrorWithContext(status, getHALErrorMessage(status)); /* Acquire the mutex; this makes certain that the handler is not being diff --git a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp index bb5c7489c2..5568161f29 100644 --- a/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp +++ b/wpilibj/src/athena/cpp/lib/NotifierJNI.cpp @@ -115,9 +115,9 @@ extern "C" { /* * Class: edu_wpi_first_wpilibj_hal_NotifierJNI * Method: initializeNotifier - * Signature: (Ljava/lang/Runnable;)J + * Signature: (Ljava/lang/Runnable;)I */ -JNIEXPORT jlong JNICALL +JNIEXPORT jint JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_initializeNotifier( JNIEnv *env, jclass, jobject func) { NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI initializeNotifier"; @@ -141,34 +141,34 @@ Java_edu_wpi_first_wpilibj_hal_NotifierJNI_initializeNotifier( notify->Start(); notify->SetFunc(env, func, mid); int32_t status = 0; - void *notifierPtr = initializeNotifier(notifierHandler, notify, &status); + HalNotifierHandle notifierHandle = initializeNotifier(notifierHandler, notify, &status); - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Handle = " << notifierHandle; NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; - if (!notifierPtr || !CheckStatus(env, status)) { + if (notifierHandle <= 0 || !CheckStatus(env, status)) { // something went wrong in HAL, clean up delete notify; } - return (jlong)notifierPtr; + return (jint)notifierHandle; } /* * Class: edu_wpi_first_wpilibj_hal_NotifierJNI * Method: cleanNotifier - * Signature: (J)V + * Signature: (I)V */ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_cleanNotifier( - JNIEnv *env, jclass, jlong notifierPtr) { + JNIEnv *env, jclass, jint notifierHandle) { NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI cleanNotifier"; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Handle = " << notifierHandle; int32_t status = 0; NotifierJNI *notify = - (NotifierJNI *)getNotifierParam((void *)notifierPtr, &status); - cleanNotifier((void *)notifierPtr, &status); + (NotifierJNI *)getNotifierParam((HalNotifierHandle)notifierHandle, &status); + cleanNotifier((HalNotifierHandle)notifierHandle, &status); NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; CheckStatus(env, status); delete notify; @@ -177,19 +177,19 @@ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_cleanNotifier( /* * Class: edu_wpi_first_wpilibj_hal_NotifierJNI * Method: updateNotifierAlarm - * Signature: (JJ)V + * Signature: (IJ)V */ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_updateNotifierAlarm( - JNIEnv *env, jclass cls, jlong notifierPtr, jlong triggerTime) { + JNIEnv *env, jclass cls, jint notifierHandle, jlong triggerTime) { NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI updateNotifierAlarm"; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Handle = " << notifierHandle; NOTIFIERJNI_LOG(logDEBUG) << "triggerTime = " << triggerTime; int32_t status = 0; - updateNotifierAlarm((void *)notifierPtr, (uint64_t)triggerTime, &status); + updateNotifierAlarm((HalNotifierHandle)notifierHandle, (uint64_t)triggerTime, &status); NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; CheckStatus(env, status); } @@ -197,17 +197,17 @@ Java_edu_wpi_first_wpilibj_hal_NotifierJNI_updateNotifierAlarm( /* * Class: edu_wpi_first_wpilibj_hal_NotifierJNI * Method: stopNotifierAlarm - * Signature: (J)V + * Signature: (I)V */ JNIEXPORT void JNICALL Java_edu_wpi_first_wpilibj_hal_NotifierJNI_stopNotifierAlarm( - JNIEnv *env, jclass cls, jlong notifierPtr) { + JNIEnv *env, jclass cls, jint notifierHandle) { NOTIFIERJNI_LOG(logDEBUG) << "Calling NOTIFIERJNI stopNotifierAlarm"; - NOTIFIERJNI_LOG(logDEBUG) << "Notifier Ptr = " << (void *)notifierPtr; + NOTIFIERJNI_LOG(logDEBUG) << "Notifier Handle = " << notifierHandle; int32_t status = 0; - stopNotifierAlarm((void *)notifierPtr, &status); + stopNotifierAlarm((HalNotifierHandle)notifierHandle, &status); NOTIFIERJNI_LOG(logDEBUG) << "Status = " << status; CheckStatus(env, status); } diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java index c47a4f5b47..57e8c6284d 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java +++ b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/Notifier.java @@ -7,6 +7,7 @@ package edu.wpi.first.wpilibj; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import edu.wpi.first.wpilibj.hal.NotifierJNI; @@ -18,7 +19,7 @@ public class Notifier { private final ReentrantLock m_processLock = new ReentrantLock(); // The C pointer to the notifier object. We don't use it directly, it is // just passed to the JNI bindings. - private long m_notifier; + AtomicInteger m_notifier = new AtomicInteger(); // The time, in microseconds, at which the corresponding handler should be // called. Has the same zero as Utility.getFPGATime(). private double m_expirationTime = 0; @@ -38,13 +39,14 @@ public class Notifier { public Process(Runnable run) { m_handler = run; - m_notifier = NotifierJNI.initializeNotifier(this); + m_notifier.set(NotifierJNI.initializeNotifier(this)); } @Override @SuppressWarnings("NoFinalizer") protected void finalize() { - NotifierJNI.cleanNotifier(m_notifier); + int handle = m_notifier.getAndSet(0); + NotifierJNI.cleanNotifier(handle); m_handlerLock.lock(); } @@ -52,7 +54,7 @@ public class Notifier { * Update the alarm hardware to reflect the next alarm. */ private void updateAlarm() { - NotifierJNI.updateNotifierAlarm(m_notifier, (long) (m_expirationTime * 1e6)); + NotifierJNI.updateNotifierAlarm(m_notifier.get(), (long) (m_expirationTime * 1e6)); } /** @@ -84,7 +86,7 @@ public class Notifier { } public void stop() { - NotifierJNI.stopNotifierAlarm(m_notifier); + NotifierJNI.stopNotifierAlarm(m_notifier.get()); // Wait for a currently executing handler to complete before returning // from stop() diff --git a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java index 6276072024..6942073cca 100644 --- a/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java +++ b/wpilibj/src/athena/java/edu/wpi/first/wpilibj/hal/NotifierJNI.java @@ -24,20 +24,20 @@ public class NotifierJNI extends JNIWrapper { /** * Initializes the notifier. */ - public static native long initializeNotifier(NotifierJNIHandlerFunction func); + public static native int initializeNotifier(NotifierJNIHandlerFunction func); /** * Deletes the notifier object when we are done with it. */ - public static native void cleanNotifier(long notifierPtr); + public static native void cleanNotifier(int notifierHandle); /** * Sets the notifier to call the callback in another triggerTime microseconds. */ - public static native void updateNotifierAlarm(long notifierPtr, long triggerTime); + public static native void updateNotifierAlarm(int notifierHandle, long triggerTime); /** * Tells the notifier to stop calling the callback. */ - public static native void stopNotifierAlarm(long notifierPtr); + public static native void stopNotifierAlarm(int notifierHandle); }