From 65a044f633ca14e6f969523c172b2e5285bb6616 Mon Sep 17 00:00:00 2001
From: Peter Johnson <johnson.peter@gmail.com>
Date: Thu, 30 Nov 2017 20:45:40 -0800
Subject: [PATCH] Fix HAL_CleanNotifier race. (#793)

This race was caused by holding a lock while calling into FRC_ChipObject,
which was waiting for the callback to exit before returning, and our
callback wanted to grab the same lock.
---
 hal/src/main/native/athena/Notifier.cpp | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hal/src/main/native/athena/Notifier.cpp b/hal/src/main/native/athena/Notifier.cpp
index ccff28851b..7a53e653e6 100644
--- a/hal/src/main/native/athena/Notifier.cpp
+++ b/hal/src/main/native/athena/Notifier.cpp
@@ -88,7 +88,7 @@ static void alarmCallback(uint32_t, void*) {
     }
   });
 
-  if (closestTrigger != UINT64_MAX) {
+  if (notifierAlarm && closestTrigger != UINT64_MAX) {
     // Simply truncate the hardware trigger time to 32-bit.
     notifierAlarm->writeTriggerTime(static_cast<uint32_t>(closestTrigger),
                                     &status);
@@ -156,16 +156,16 @@ void HAL_CleanNotifier(HAL_NotifierHandle notifierHandle, int32_t* status) {
   notifier->cond.notify_all();
 
   if (notifierRefCount.fetch_sub(1) == 1) {
-    std::lock_guard<wpi::mutex> lock(notifierMutex);
     // if this was the last notifier, clean up alarm and manager
-    if (notifierAlarm) {
-      notifierAlarm->writeEnable(false, status);
-      notifierAlarm = nullptr;
-    }
-    if (notifierManager) {
-      notifierManager->disable(status);
-      notifierManager = nullptr;
-    }
+    // the notifier can call back into our callback, so don't hold the lock
+    // here (the atomic fetch_sub will prevent multiple parallel entries
+    // into this function)
+    if (notifierAlarm) notifierAlarm->writeEnable(false, status);
+    if (notifierManager) notifierManager->disable(status);
+
+    std::lock_guard<wpi::mutex> lock(notifierMutex);
+    notifierAlarm = nullptr;
+    notifierManager = nullptr;
     closestTrigger = UINT64_MAX;
   }
 }