Fixes HandleResource classes (#95)

Issues were found when adding unit testing. The classes should now work
properly.

hal/lib/athena/handles/IndexedHandleResource.h

@@ -18,6 +18,9 @@
 
 namespace hal {
 
+constexpr int32_t IndexedResourceIndexOutOfRange = -1;
+constexpr int32_t IndexedResourceNotAllocated = -2;
+
 /**
  * The IndexedHandleResource class is a way to track handles. This version
  * allows a limited number of handles that are allocated by index.
@@ -33,18 +36,20 @@ namespace hal {
 template <typename THandle, typename TStruct, int16_t size,
           HalHandleEnum enumValue>
 class IndexedHandleResource {
+  friend class IndexedHandleResourceTest;
+
  public:
   IndexedHandleResource(const IndexedHandleResource&) = delete;
   IndexedHandleResource operator=(const IndexedHandleResource&) = delete;
   IndexedHandleResource();
   THandle Allocate(int16_t index, const TStruct& toSet);
-  TStruct Get(THandle handle);
+  TStruct Get(THandle handle, int32_t* status);
   void Free(THandle handle);
 
  private:
   TStruct m_structures[size];
   bool m_allocated[size];
-  priority_recursive_mutex m_handleMutexes[size];
+  priority_mutex m_handleMutexes[size];
 };
 
 template <typename THandle, typename TStruct, int16_t size,
@@ -62,8 +67,8 @@ template <typename THandle, typename TStruct, int16_t size,
 THandle IndexedHandleResource<THandle, TStruct, size, enumValue>::Allocate(
     int16_t index, const TStruct& toSet) {
   // don't aquire the lock if we can fail early.
-  if (index < 0 || index > size) return HAL_HANDLE_INDEX_OUT_OF_RANGE;
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutexes[index]);
+  if (index < 0 || index >= size) return HAL_HANDLE_INDEX_OUT_OF_RANGE;
+  std::lock_guard<priority_mutex> sync(m_handleMutexes[index]);
   // check for allocation, otherwise allocate and return a valid handle
   if (m_allocated[index]) return HAL_HANDLE_ALREADY_ALLOCATED;
   m_allocated[index] = true;
@@ -74,13 +79,19 @@ THandle IndexedHandleResource<THandle, TStruct, size, enumValue>::Allocate(
 template <typename THandle, typename TStruct, int16_t size,
           HalHandleEnum enumValue>
 TStruct IndexedHandleResource<THandle, TStruct, size, enumValue>::Get(
-    THandle handle) {
+    THandle handle, int32_t* status) {
   // get handle index, and fail early if index out of range or wrong handle
   int16_t index = getHandleTypedIndex(handle, enumValue);
-  if (index < 0 || index > size) return nullptr;
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutexes[index]);
+  if (index < 0 || index >= size) {
+    *status = IndexedResourceIndexOutOfRange;
+    return TStruct();
+  }
+  std::lock_guard<priority_mutex> sync(m_handleMutexes[index]);
   // check for already deallocated handle, then return structure
-  if (!m_allocated[index]) return nullptr;
+  if (!m_allocated[index]) {
+    *status = IndexedResourceNotAllocated;
+    return TStruct();
+  }
   return m_structures[index];
 }
 
@@ -90,9 +101,9 @@ void IndexedHandleResource<THandle, TStruct, size, enumValue>::Free(
     THandle handle) {
   // get handle index, and fail early if index out of range or wrong handle
   int16_t index = getHandleTypedIndex(handle, enumValue);
-  if (index < 0 || index > size) return;
+  if (index < 0 || index >= size) return;
   // lock and deallocated handle
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutexes[index]);
-  m_allocated(index) = false;
+  std::lock_guard<priority_mutex> sync(m_handleMutexes[index]);
+  m_allocated[index] = false;
 }
 }

hal/lib/athena/handles/LimitedHandleResource.h

@@ -18,6 +18,9 @@
 
 namespace hal {
 
+constexpr int32_t LimitedResourceIndexOutOfRange = -1;
+constexpr int32_t LimitedResourceNotAllocated = -2;
+
 /**
  * The LimitedHandleResource class is a way to track handles. This version
  * allows a limited number of handles that are allocated sequentially.
@@ -31,19 +34,21 @@ namespace hal {
 template <typename THandle, typename TStruct, int16_t size,
           HalHandleEnum enumValue>
 class LimitedHandleResource {
+  friend class LimitedHandleResourceTest;
+
  public:
   LimitedHandleResource(const LimitedHandleResource&) = delete;
   LimitedHandleResource operator=(const LimitedHandleResource&) = delete;
   LimitedHandleResource();
   THandle Allocate(const TStruct& toSet);
-  TStruct Get(THandle handle);
+  TStruct Get(THandle handle, int32_t* status);
   void Free(THandle handle);
 
  private:
   TStruct m_structures[size];
   bool m_allocated[size];
-  priority_recursive_mutex m_handleMutexes[size];
-  priority_recursive_mutex m_allocateMutex;
+  priority_mutex m_handleMutexes[size];
+  priority_mutex m_allocateMutex;
 };
 
 template <typename THandle, typename TStruct, int16_t size,
@@ -61,13 +66,13 @@ template <typename THandle, typename TStruct, int16_t size,
 THandle LimitedHandleResource<THandle, TStruct, size, enumValue>::Allocate(
     const TStruct& toSet) {
   // globally lock to loop through indices
-  std::lock_guard<priority_recursive_mutex> sync(m_allocateMutex);
+  std::lock_guard<priority_mutex> sync(m_allocateMutex);
   int16_t i;
   for (i = 0; i < size; i++) {
     if (m_allocated[i] == false) {
       // if a false index is found, grab its specific mutex
       // and allocate it.
-      std::lock_guard<priority_recursive_mutex> sync(m_handleMutexes[i]);
+      std::lock_guard<priority_mutex> sync(m_handleMutexes[i]);
       m_allocated[i] = true;
       m_structures[i] = toSet;
       return (THandle)createHandle(i, enumValue);
@@ -79,13 +84,20 @@ THandle LimitedHandleResource<THandle, TStruct, size, enumValue>::Allocate(
 template <typename THandle, typename TStruct, int16_t size,
           HalHandleEnum enumValue>
 TStruct LimitedHandleResource<THandle, TStruct, size, enumValue>::Get(
-    THandle handle) {
+    THandle handle, int32_t* status) {
+  *status = 0;
   // get handle index, and fail early if index out of range or wrong handle
   int16_t index = getHandleTypedIndex(handle, enumValue);
-  if (index < 0 || index > size) return nullptr;
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutexes[index]);
+  if (index < 0 || index >= size) {
+    *status = LimitedResourceIndexOutOfRange;
+    return TStruct();
+  }
+  std::lock_guard<priority_mutex> sync(m_handleMutexes[index]);
   // check for already deallocated handle, then return structure
-  if (!m_allocated[index]) return nullptr;
+  if (!m_allocated[index]) {
+    *status = LimitedResourceNotAllocated;
+    return TStruct();
+  }
   return m_structures[index];
 }
 
@@ -95,10 +107,10 @@ void LimitedHandleResource<THandle, TStruct, size, enumValue>::Free(
     THandle handle) {
   // get handle index, and fail early if index out of range or wrong handle
   int16_t index = getHandleTypedIndex(handle, enumValue);
-  if (index < 0 || index > size) return nullptr;
+  if (index < 0 || index >= size) return;
   // lock and deallocated handle
-  std::lock_guard<priority_recursive_mutex> sync(m_allocateMutex);
-  std::lock_guard<priority_recursive_mutex> lock(m_handleMutexes[index]);
+  std::lock_guard<priority_mutex> sync(m_allocateMutex);
+  std::lock_guard<priority_mutex> lock(m_handleMutexes[index]);
   m_allocated[index] = false;
 }
 }

hal/lib/athena/handles/UnlimitedHandleResource.h

@@ -14,6 +14,7 @@
 
 #include "HAL/Handles.h"
 #include "HAL/cpp/priority_mutex.h"
+#include "handles/HandlesInternal.h"
 
 namespace hal {
 
@@ -33,6 +34,8 @@ namespace hal {
  */
 template <typename THandle, typename TStruct, HalHandleEnum enumValue>
 class UnlimitedHandleResource {
+  friend class UnlimitedHandleResourceTest;
+
  public:
   UnlimitedHandleResource(const UnlimitedHandleResource&) = delete;
   UnlimitedHandleResource operator=(const UnlimitedHandleResource&) = delete;
@@ -43,13 +46,13 @@ class UnlimitedHandleResource {
 
  private:
   std::vector<std::shared_ptr<TStruct>> m_structures;
-  priority_recursive_mutex m_handleMutex;
+  priority_mutex m_handleMutex;
 };
 
 template <typename THandle, typename TStruct, HalHandleEnum enumValue>
 THandle UnlimitedHandleResource<THandle, TStruct, enumValue>::Allocate(
     std::shared_ptr<TStruct> structure) {
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutex);
+  std::lock_guard<priority_mutex> sync(m_handleMutex);
   int16_t i;
   for (i = 0; i < m_structures.size(); i++) {
     if (m_structures[i] == nullptr) {
@@ -66,18 +69,18 @@ THandle UnlimitedHandleResource<THandle, TStruct, enumValue>::Allocate(
 template <typename THandle, typename TStruct, HalHandleEnum enumValue>
 std::shared_ptr<TStruct>
 UnlimitedHandleResource<THandle, TStruct, enumValue>::Get(THandle handle) {
-  int16_t index = getHandleTypedIndex(handle);
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutex);
-  if (index < 0 || index > m_structures.size()) return nullptr;
+  int16_t index = getHandleTypedIndex(handle, enumValue);
+  std::lock_guard<priority_mutex> sync(m_handleMutex);
+  if (index < 0 || index >= m_structures.size()) return nullptr;
   return m_structures[index];
 }
 
 template <typename THandle, typename TStruct, HalHandleEnum enumValue>
 void UnlimitedHandleResource<THandle, TStruct, enumValue>::Free(
     THandle handle) {
-  int16_t index = getHandleTypedIndex(handle);
-  std::lock_guard<priority_recursive_mutex> sync(m_handleMutex);
-  if (index < 0 || index > m_structures.size()) return nullptr;
+  int16_t index = getHandleTypedIndex(handle, enumValue);
+  std::lock_guard<priority_mutex> sync(m_handleMutex);
+  if (index < 0 || index >= m_structures.size()) return;
   m_structures[index].reset();
 }
 }