Fix error type check in InternalSetThreadPriority (#16959)

The pthread_setschedparam returns error code as a return value and not
in the errno as the existing code supposes. I have found it when a test
in the lab has failed to set the priority and asserted in this code, but
reported errno as 0.

Also fix the flaky DuplicateHandle test7 that was failing rarely due to the
fact that the secondary thread it has created didn't wait until the main thread
finished fiddling with the priority of that thread.

diff --git a/src/pal/src/thread/thread.cpp b/src/pal/src/thread/thread.cpp
index c6bbfb8..bc06d2f 100644 (file)
--- a/src/pal/src/thread/thread.cpp
+++ b/src/pal/src/thread/thread.cpp
@@ -1101,7 +1101,8 @@ CorUnix::InternalSetThreadPriority(
     PAL_ERROR palError = NO_ERROR;
     CPalThread *pTargetThread = NULL;
     IPalObject *pobjThread = NULL;
-    
+
+    int st;
     int policy;
     struct sched_param schedParam;
     int max_priority;
@@ -1237,14 +1238,11 @@ CorUnix::InternalSetThreadPriority(
           iNewPriority, schedParam.sched_priority);
 
     /* Finally, set the new priority into place */
-    if (pthread_setschedparam(
-            pTargetThread->GetPThreadSelf(),
-            policy,
-            &schedParam
-            ) != 0)
+    st = pthread_setschedparam(pTargetThread->GetPThreadSelf(), policy, &schedParam);
+    if (st != 0)
     {
 #if SET_SCHEDPARAM_NEEDS_PRIVS
-        if (EPERM == errno)
+        if (EPERM == st)
         {
             // UNIXTODO: Should log a warning to the event log
             TRACE("Caller does not have OS privileges to call pthread_setschedparam\n");
@@ -1253,7 +1251,7 @@ CorUnix::InternalSetThreadPriority(
         }
 #endif
         
-        ASSERT("Unable to set thread priority (errno %d)\n", errno);
+        ASSERT("Unable to set thread priority to %d (error %d)\n", (int)posix_priority, st);
         palError = ERROR_INTERNAL_ERROR;
         goto InternalSetThreadPriorityExit;
     }

diff --git a/src/pal/tests/palsuite/threading/DuplicateHandle/test7/test7.cpp b/src/pal/tests/palsuite/threading/DuplicateHandle/test7/test7.cpp
index 0477291..bd09283 100644 (file)
--- a/src/pal/tests/palsuite/threading/DuplicateHandle/test7/test7.cpp
+++ b/src/pal/tests/palsuite/threading/DuplicateHandle/test7/test7.cpp
@@ -25,6 +25,7 @@ int __cdecl main(int argc, char* argv[])
     HANDLE  hDupThread;  
     DWORD   dwThreadId = 0;
     LPTHREAD_START_ROUTINE lpStartAddress =  &CreateTestThread;
+    HANDLE  hSyncEvent;
 
     int threadPriority;
     int duplicatePriority;
@@ -36,11 +37,25 @@ int __cdecl main(int argc, char* argv[])
         return (FAIL);
     }
     
+    LPSECURITY_ATTRIBUTES lpEventAttributes = NULL;
+    BOOL bManualReset = TRUE;
+    BOOL bInitialState = FALSE;
+    hSyncEvent = CreateEvent(lpEventAttributes,
+                             bManualReset,
+                             bInitialState,
+                             NULL);
+
+    if (hSyncEvent == NULL)
+    {
+        Fail("ERROR:%u: Unable to create sync event.\n",
+             GetLastError());
+    }
+
     /* Create a thread.*/
     hThread = CreateThread(NULL,            /* SD*/
                           (DWORD)0,         /* initial stack size*/
                           lpStartAddress,   /* thread function*/
-                          NULL,             /* thread argument*/
+                          (VOID*)hSyncEvent,/* thread argument*/
                           (DWORD)0,         /* creation option*/
                           &dwThreadId);     /* thread identifier*/
     if (hThread == NULL)
@@ -123,6 +138,13 @@ int __cdecl main(int argc, char* argv[])
         Fail("");
     }
 
+    /* Signal the helper thread that it can shut down */
+    if (!SetEvent(hSyncEvent))
+    {
+        Fail("ERROR:%u: Failed to set event.\n",
+             GetLastError());
+    }
+
     /* Wait on the original thread.*/
     if((WaitForSingleObject(hThread, 100)) != WAIT_OBJECT_0)
     {
@@ -136,14 +158,21 @@ int __cdecl main(int argc, char* argv[])
     }
 
     /* Clean-up thread and Terminate the PAL.*/
+    CloseHandle(hSyncEvent);
     CloseHandle(hThread);
     CloseHandle(hDupThread);
     PAL_Terminate();
     return PASS;
 }
 
-/*Thread testing function, only return '0'*/
+/*Thread testing function*/
 DWORD PALAPI CreateTestThread(LPVOID lpParam)
 {
+    HANDLE hSyncEvent = (HANDLE)lpParam;
+
+    /* Wait until the main thread signals that this helper thread should shut down */
+    WaitForSingleObject(hSyncEvent, INFINITE);
+
     return (DWORD)0;
 }
+