🐛 Refactor task_notify_when_deleting to support task_to_notify dying before target_task (purduesigbots#100)

#### Summary:
- Increments `configNUM_THREAD_LOCAL_STORAGE_POINTERS` again
- Adds another linked list to thread local storage that will keep track of the tasks to whose death events `task_to_notify` is subscribed
- When a task dies, the `task_delete_hook` now iterates through the task's list of subscriptions and removes references to itself from the subscriptions' lists of subscribers

#### Motivation:
Previously, the case in which `task_to_notify` dies before `target_task` was undefined behavior, because `target_task` would attempt to notify `task_to_notify` even if it's already been cleaned up. The naive solution of checking the `task_to_notify`'s state directly would not have worked because in certain cases (as with the competition tasks), the TCB can be reused.

More information regarding the motivation behind this change can be found at purduesigbots#86, OkapiLib/OkapiLib#249, and OkapiLib/OkapiLib#321

##### References (optional):
Closes purduesigbots#86 

#### Test Plan:
- [x] execute test plan in src/tests/task_notify_when_deleting.c (regression test)
(the following are adapted from purduesigbots#100 (comment). thanks @Octogonapus)
- [x] delete `task_to_notify` and then delete `target_task`, verify nothing goes horrendously wrong
- [x] repeat previous test with OkapiLib where an internal task is the `task_to_notify`

#### Commits:
* begin task_notify_when_deleting refactor

* add nullcheck

needed to check whether subscriptions_list is null in
unsubscribe_hook_cb to handle the "normal operation" case, where
target_task dies before task_to_notify

* delete subscriptions list anyway

* look for correct task in subscriptions_ll

* mutex guard task_notify_when_deleting functions, cleanup includes

* stick with one method of initializing mutex

* Revert "stick with one method of initializing mutex"

This reverts commit 154b969.

* stick with the other method of initializing mutex

Author: HotelCalifornia

include/pros/apix.h

@@ -60,8 +60,6 @@ bool task_abort_delay(task_t task);
  * task_notify_ext(task_to_notify, value, action, NULL) when target_task is
  * deleted.
  *
- * NOTE: This facility does not support the case when task_to_notify
- *       dies before target_task
  *
  * \param target_task
  *				The task being watched for deletion

include/rtos/FreeRTOSConfig.h

@@ -141,7 +141,7 @@
 #define configUSE_NEWLIB_REENTRANT              1
 #define configSTACK_DEPTH_TYPE                  size_t
 
-#define configNUM_THREAD_LOCAL_STORAGE_POINTERS 1
+#define configNUM_THREAD_LOCAL_STORAGE_POINTERS 2
 
 /* Include the query-heap CLI command to query the free heap space. */
 #define configINCLUDE_QUERY_HEAP_COMMAND        1

src/rtos/task_notify_when_deleting.c

@@ -1,10 +1,20 @@
+#include "kapi.h"
 #include "common/linkedlist.h"
-#include "rtos/task.h"
+
+// NOTE: can't just include task.h because of redefinition that goes on in kapi
+//       include chain, so we just prototype what we need here
+void *pvTaskGetThreadLocalStoragePointer( task_t xTaskToQuery, int32_t xIndex );
+void vTaskSetThreadLocalStoragePointer( task_t xTaskToSet, int32_t xIndex, void *pvValue );
+
 #include "rtos/tcb.h"
 
-// This increments configNUM_THREAD_LOCAL_STORAGE_POINTERS by 1
+// This increments configNUM_THREAD_LOCAL_STORAGE_POINTERS by 2
+
+#define SUBSCRIBERS_TLSP_IDX 0
+#define SUBSCRIPTIONS_TLSP_IDX 1
 
-#define TLSP_IDX 0
+static static_sem_s_t task_notify_when_deleting_mutex_buf;
+static mutex_t task_notify_when_deleting_mutex;
 
 struct notify_delete_action {
   task_t task_to_notify;
@@ -36,20 +46,51 @@ static struct notify_delete_action* _find_task(linked_list_s_t* ll, task_t task)
   return args.found_action;
 }
 
+void task_notify_when_deleting_init() {
+  task_notify_when_deleting_mutex = mutex_create_static(&task_notify_when_deleting_mutex_buf);
+}
+
 void task_notify_when_deleting(task_t target_task, task_t task_to_notify,
                                uint32_t value, notify_action_e_t notify_action) {
   task_to_notify = (task_to_notify == NULL) ? pxCurrentTCB : task_to_notify;
   target_task = (target_task == NULL) ? pxCurrentTCB : target_task;
 
-  // It doesn't make sense for a task to notify itself, and make sure that neither task is NULL (implying that scheduler hasn't started yet)
+  // It doesn't make sense for a task to notify itself, and make sure that
+  // neither task is NULL (implying that scheduler hasn't started yet)
   if (task_to_notify == target_task || !task_to_notify || !target_task) {
     return;
   }
 
-  linked_list_s_t* target_ll = pvTaskGetThreadLocalStoragePointer(target_task, TLSP_IDX);
+  mutex_take(task_notify_when_deleting_mutex, TIMEOUT_MAX);
+
+  // task_to_notify maintains a list of the tasks whose deletion it cares about.
+  // This will allow us to unsubscribe from notification if/when task_to_notify
+  // is deleted
+  linked_list_s_t* subscriptions_ll = pvTaskGetThreadLocalStoragePointer(task_to_notify, SUBSCRIPTIONS_TLSP_IDX);
+  if (subscriptions_ll == NULL) {
+    subscriptions_ll = linked_list_init();
+    vTaskSetThreadLocalStoragePointer(task_to_notify, SUBSCRIPTIONS_TLSP_IDX, subscriptions_ll);
+  }
+  if (subscriptions_ll != NULL) {
+    // check whether task_to_notify is already subscribed to target_task. if so,
+    // do nothing
+    ll_node_s_t* it = subscriptions_ll->head;
+    bool found = false;
+    while (it != NULL && !found) {
+      found = it->payload.data == target_task;
+      it = it->next;
+    }
+    if (!found) {
+      linked_list_prepend_data(subscriptions_ll, target_task);
+    }
+  }
+
+  // similarly, target_task maintains a list of the tasks it needs to notify
+  // when being deleted
+  linked_list_s_t* target_ll = pvTaskGetThreadLocalStoragePointer(target_task, SUBSCRIBERS_TLSP_IDX);
   if (target_ll == NULL) {
     target_ll = linked_list_init();
-    vTaskSetThreadLocalStoragePointer(target_task, TLSP_IDX, target_ll);
+    vTaskSetThreadLocalStoragePointer(target_task, SUBSCRIBERS_TLSP_IDX, target_ll);
   }
 
   if (target_ll != NULL) {
@@ -72,6 +113,7 @@ void task_notify_when_deleting(task_t target_task, task_t task_to_notify,
       action->value = value;
     }
   }
+  mutex_give(task_notify_when_deleting_mutex);
 }
 
 // NOTE: this code is untested, probably works, but also has a terrible name (task_notify_when_deleting_unsubscribe)
@@ -93,6 +135,15 @@ void task_notify_when_deleting(task_t target_task, task_t task_to_notify,
 //   }
 // }
 
+static void unsubscribe_hook_cb(ll_node_s_t* node, void* task_to_remove) {
+  task_t subscription = node->payload.data;
+
+  linked_list_s_t* subscriptions_list = pvTaskGetThreadLocalStoragePointer(subscription, SUBSCRIBERS_TLSP_IDX);
+  if (subscriptions_list != NULL) {
+    linked_list_remove_data(subscriptions_list, task_to_remove);
+  }
+}
+
 static void delete_hook_cb(ll_node_s_t* node, void* ignore) {
   struct notify_delete_action* action = node->payload.data;
   if (action != NULL) {
@@ -103,10 +154,20 @@ static void delete_hook_cb(ll_node_s_t* node, void* ignore) {
 }
 
 void task_notify_when_deleting_hook(task_t task) {
-  linked_list_s_t* ll = pvTaskGetThreadLocalStoragePointer(task, TLSP_IDX);
+  mutex_take(task_notify_when_deleting_mutex, TIMEOUT_MAX);
+  // if this task was subscribed to any other task deletion events, unsubscribe
+  linked_list_s_t* ll = pvTaskGetThreadLocalStoragePointer(task, SUBSCRIPTIONS_TLSP_IDX);
+  if (ll != NULL) {
+    linked_list_foreach(ll, unsubscribe_hook_cb, task);
+    linked_list_free(ll);
+    vTaskSetThreadLocalStoragePointer(task, SUBSCRIPTIONS_TLSP_IDX, NULL);
+  }
+  // notify subscribed tasks of this task's deletion
+  ll = pvTaskGetThreadLocalStoragePointer(task, SUBSCRIBERS_TLSP_IDX);
   if (ll != NULL) {
     linked_list_foreach(ll, delete_hook_cb, NULL);
     linked_list_free(ll);
-    vTaskSetThreadLocalStoragePointer(task, TLSP_IDX, NULL); // for good measure
+    vTaskSetThreadLocalStoragePointer(task, SUBSCRIBERS_TLSP_IDX, NULL); // for good measure
   }
+  mutex_give(task_notify_when_deleting_mutex);
 }

src/system/rtos_hooks.c

@@ -44,6 +44,9 @@ void rtos_initialize() {
 	portDISABLE_INTERRUPTS();
 
 	vPortInstallFreeRTOSVectorTable();
+
+	void task_notify_when_deleting_init();
+	task_notify_when_deleting_init();
 }
 
 extern void FreeRTOS_Tick_Handler(void);