Merge pull request #853 from leapmotion/ref-setthreadpriority

Make `SetThreadPriority` protected

src/autowiring/BasicThread.h

@@ -69,6 +69,48 @@ class BasicThread:
   BasicThread(const char* pName = nullptr);
   virtual ~BasicThread(void);
 
+  /// \internal Only implemented on Windows (as of 0.4.1).
+  /// <summary>
+  /// Boosts thread priority while an instance of this type exists.
+  /// </summary>
+  /// <remarks>
+  /// The thread priority is changed when the requested priority is higher than the
+  /// current priority.  The destructor automatically restores the previous priority.
+  /// This class cannot be moved or copied, in order to guarantee proper RAII.
+  /// </remarks>
+  class ElevatePriority {
+  public:
+    /// Creating an ElevatePriority object as a member of a BasicThread instance
+    /// elevates the priority of the thread if the specified priority is higher
+    /// then the current thread priority. Destroy this ElevatePriority instance
+    /// to restore the normal thread priority.
+    ElevatePriority(ElevatePriority&) = delete;
+
+    /// Elevates the priority of a BasicThread instance if the specified priority is higher
+    /// then the current thread priority. Destroy this ElevatePriority instance
+    /// to restore the normal thread priority.
+    ElevatePriority(BasicThread& thread, ThreadPriority priority) :
+      m_oldPriority(thread.m_priority),
+      m_thread(thread)
+    {
+      // Elevate if the new level is higher than the old level:
+      if (priority > m_oldPriority)
+        m_thread.SetThreadPriority(priority);
+    }
+
+    /// Destroying this object returns the thread to its previous priority
+    /// level.
+    ~ElevatePriority(void) {
+      // Delevate if the old level is lower than the current level:
+      if (m_thread.m_priority > m_oldPriority)
+        m_thread.SetThreadPriority(m_oldPriority);
+    }
+
+  private:
+    ThreadPriority m_oldPriority;
+    BasicThread& m_thread;
+  };
+
 protected:
   // Internally held thread status block.  This has to be a shared pointer because we need to signal
   // the held state condition after releasing all shared pointers to ourselves, and this could mean
@@ -97,8 +139,6 @@ class BasicThread:
   /// </remarks>
   void SetCurrentThreadName(void) const;
 
-private:
-  /// Only implemented on Windows (as of version 0.4.1).
   /// <summary>
   /// Sets the thread priority of this thread
   /// </summary>
@@ -108,7 +148,6 @@ class BasicThread:
   /// </remarks>
   void SetThreadPriority(ThreadPriority threadPriority);
 
-protected:
   /// <summary>
   /// Recovers a general lock used to synchronize entities in this thread internally.
   /// </summary>
@@ -135,48 +174,6 @@ class BasicThread:
   /// <param name="refTracker">A reference tracker held for as long as the cleanup operation is incomplete</param>
   virtual void DoRunLoopCleanup(std::shared_ptr<CoreContext>&& ctxt, std::shared_ptr<CoreObject>&& refTracker);
 
-  /// \internal Only implemented on Windows (as of 0.4.1).
-  /// <summary>
-  /// Boosts thread priority while an instance of this type exists.
-  /// </summary>
-  /// <remarks>
-  /// The thread priority is changed when the requested priority is higher than the
-  /// current priority.  The destructor automatically restores the previous priority.
-  /// This class cannot be moved or copied, in order to guarantee proper RAII.
-  /// </remarks>
-  class ElevatePriority {
-  public:
-    /// Creating an ElevatePriority object as a member of a BasicThread instance
-    /// elevates the priority of the thread if the specified priority is higher
-    /// then the current thread priority. Destroy this ElevatePriority instance
-    /// to restore the normal thread priority.
-    ElevatePriority(ElevatePriority&) = delete;
-
-    /// Elevates the priority of a BasicThread instance if the specified priority is higher
-    /// then the current thread priority. Destroy this ElevatePriority instance
-    /// to restore the normal thread priority.
-    ElevatePriority(BasicThread& thread, ThreadPriority priority) :
-      m_oldPriority(thread.m_priority),
-      m_thread(thread)
-    {
-      // Elevate if the new level is higher than the old level:
-      if(priority > m_oldPriority)
-        m_thread.SetThreadPriority(priority);
-    }
-
-    /// Destroying this object returns the thread to its previous priority
-    /// level.
-    ~ElevatePriority(void) {
-      // Delevate if the old level is lower than the current level:
-      if(m_thread.m_priority > m_oldPriority)
-        m_thread.SetThreadPriority(m_oldPriority);
-    }
-
-  private:
-    ThreadPriority m_oldPriority;
-    BasicThread& m_thread;
-  };
-
   /// <summary>
   /// Waits until the specified lambda function returns true or the thread shuts down.
   /// </summary>
@@ -208,6 +205,11 @@ class BasicThread:
   bool DoAdditionalWait(std::chrono::nanoseconds timeout) override;
 
 public:
+  /// <returns>
+  /// The current thread priority
+  /// </returns>
+  ThreadPriority GetThreadPriority(void) const { return m_priority; }
+
   /// <returns>
   /// True if this thread has transitioned to a completed state
   /// </returns>

src/autowiring/CoreThreadLinux.cpp

@@ -28,7 +28,7 @@ void BasicThread::GetThreadTimes(std::chrono::milliseconds& kernelTime, std::chr
 
 void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
   struct sched_param param = { 0 };
-  int policy = SCHED_RR;
+  int policy = SCHED_OTHER;
   int percent = 0;
   int min_priority;
 
@@ -46,6 +46,7 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
   case ThreadPriority::BelowNormal:
     percent = 20;
     break;
+  case ThreadPriority::Default:
   case ThreadPriority::Normal:
     percent = 50;
     break;
@@ -56,13 +57,15 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
     percent = 83;
     break;
   case ThreadPriority::TimeCritical:
+  case ThreadPriority::Multimedia:
     percent = 100;
     break;
   default:
-    throw std::runtime_error("Attempted to assign an unrecognized thread priority");
+    throw std::invalid_argument("Attempted to assign an unrecognized thread priority");
   }
   min_priority = sched_get_priority_min(policy);
+  pthread_getschedparam(m_state->m_thisThread.native_handle(), &policy, &param);
   param.sched_priority = min_priority + (percent * (sched_get_priority_max(policy) - min_priority) + 50) / 100;
-
   pthread_setschedparam(m_state->m_thisThread.native_handle(), policy, &param);
+  m_priority = threadPriority;
 }

src/autowiring/CoreThreadMac.cpp

@@ -54,7 +54,7 @@ void BasicThread::GetThreadTimes(std::chrono::milliseconds& kernelTime, std::chr
 
 void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
   struct sched_param param = { 0 };
-  int policy = SCHED_RR;
+  int policy = SCHED_OTHER;
   int percent = 0;
 
   switch (threadPriority) {
@@ -68,6 +68,7 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
     percent = 20;
     break;
   case ThreadPriority::Normal:
+  case ThreadPriority::Default:
     percent = 50;
     break;
   case ThreadPriority::AboveNormal:
@@ -77,12 +78,14 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
     percent = 83;
     break;
   case ThreadPriority::TimeCritical:
+  case ThreadPriority::Multimedia:
     percent = 100;
     break;
   default:
-    throw std::runtime_error("Attempted to assign an unrecognized thread priority");
+    throw std::invalid_argument("Attempted to assign an unrecognized thread priority");
   }
+  pthread_getschedparam(m_state->m_thisThread.native_handle(), &policy, &param);
   param.sched_priority = PTHREAD_MIN_PRIORITY + (percent*(PTHREAD_MAX_PRIORITY - PTHREAD_MIN_PRIORITY) + 50) / 100;
-
   pthread_setschedparam(m_state->m_thisThread.native_handle(), policy, &param);
+  m_priority = threadPriority;
 }

src/autowiring/CoreThreadWin.cpp

@@ -73,6 +73,7 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
     nPriority = THREAD_PRIORITY_BELOW_NORMAL;
     break;
   case ThreadPriority::Normal:
+  case ThreadPriority::Default:
     nPriority = THREAD_PRIORITY_NORMAL;
     break;
   case ThreadPriority::AboveNormal:
@@ -93,13 +94,14 @@ void BasicThread::SetThreadPriority(ThreadPriority threadPriority) {
     nPriority = THREAD_PRIORITY_TIME_CRITICAL;
     break;
   default:
-    throw std::runtime_error("Attempted to assign an unrecognized thread priority");
+    throw std::invalid_argument("Attempted to assign an unrecognized thread priority");
   }
 
   ::SetThreadPriority(
     m_state->m_thisThread.native_handle(),
     nPriority
   );
+  m_priority = threadPriority;
 }
 
 std::chrono::steady_clock::time_point BasicThread::GetCreationTime(void) {

src/autowiring/test/CoreThreadTest.cpp

@@ -635,3 +635,12 @@ TEST_F(CoreThreadTest, LambdaHoldAfterTermination) {
   }
   ASSERT_TRUE(childWeak.expired()) << "Child context leaked due to lambda pending in teardown";
 }
+
+TEST_F(CoreThreadTest, CanElevateAnyPriority) {
+  AutoRequired<CoreThread> ct;
+
+  for (int i = (int)ThreadPriority::Default; i < (int)ThreadPriority::Multimedia; i++) {
+    BasicThread::ElevatePriority ep{ *ct, (ThreadPriority)i };
+    ASSERT_EQ((ThreadPriority)i, ct->GetThreadPriority());
+  }
+}