Change new thread's affinity after thread starts, from the same threa…

…d, as a workaround for Snap (dotnet#40205) - Snap's default strict confinement doesn't allow setting the affinity of a different thread, and currently doesn't allow `sched_setaffinity(<nonzeroPid>, ...)`, which pthread implementation calls - Switched to use sched_setaffinity(0, ...) where appropriate Fix for dotnet#1634 in master
Jacksondr5 · Aug 10, 2020 · 0e19aa2 · 0e19aa2
1 parent f36e6b2
commit 0e19aa2
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 46 deletions.
diff --git a/src/coreclr/src/gc/unix/config.gc.h.in b/src/coreclr/src/gc/unix/config.gc.h.in
@@ -18,7 +18,8 @@
 #cmakedefine01 HAVE_PTHREAD_CONDATTR_SETCLOCK
 #cmakedefine01 HAVE_MACH_ABSOLUTE_TIME
 #cmakedefine01 HAVE_SCHED_GETAFFINITY
-#cmakedefine01 HAVE_PTHREAD_GETAFFINITY_NP
+#cmakedefine01 HAVE_SCHED_SETAFFINITY
+#cmakedefine01 HAVE_PTHREAD_SETAFFINITY_NP
 #cmakedefine01 HAVE_PTHREAD_NP_H
 #cmakedefine01 HAVE_CPUSET_T
 #cmakedefine01 HAVE__SC_AVPHYS_PAGES

diff --git a/src/coreclr/src/gc/unix/configure.cmake b/src/coreclr/src/gc/unix/configure.cmake
@@ -86,6 +86,7 @@ check_cxx_source_runs("
 
 
 check_library_exists(c sched_getaffinity "" HAVE_SCHED_GETAFFINITY)
+check_library_exists(c sched_setaffinity "" HAVE_SCHED_SETAFFINITY)
 check_library_exists(pthread pthread_create "" HAVE_LIBPTHREAD)
 
 if (HAVE_LIBPTHREAD)
@@ -94,7 +95,7 @@ elseif (HAVE_PTHREAD_IN_LIBC)
   set(PTHREAD_LIBRARY c)
 endif()
 
-check_library_exists(${PTHREAD_LIBRARY} pthread_getaffinity_np "" HAVE_PTHREAD_GETAFFINITY_NP)
+check_library_exists(${PTHREAD_LIBRARY} pthread_setaffinity_np "" HAVE_PTHREAD_SETAFFINITY_NP)
 
 check_cxx_symbol_exists(_SC_PHYS_PAGES unistd.h HAVE__SC_PHYS_PAGES)
 check_cxx_symbol_exists(_SC_AVPHYS_PAGES unistd.h HAVE__SC_AVPHYS_PAGES)

diff --git a/src/coreclr/src/gc/unix/gcenv.unix.cpp b/src/coreclr/src/gc/unix/gcenv.unix.cpp
@@ -985,19 +985,32 @@ size_t GCToOSInterface::GetCacheSizePerLogicalCpu(bool trueSize)
 //  true if setting the affinity was successful, false otherwise.
 bool GCToOSInterface::SetThreadAffinity(uint16_t procNo)
 {
-#if HAVE_PTHREAD_GETAFFINITY_NP
+#if HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP
     cpu_set_t cpuSet;
     CPU_ZERO(&cpuSet);
     CPU_SET((int)procNo, &cpuSet);
 
+    // Snap's default strict confinement does not allow sched_setaffinity(<nonzeroPid>, ...) without manually connecting the
+    // process-control plug. sched_setaffinity(<currentThreadPid>, ...) is also currently not allowed, only
+    // sched_setaffinity(0, ...). pthread_setaffinity_np(pthread_self(), ...) seems to call
+    // sched_setaffinity(<currentThreadPid>, ...) in at least one implementation, and does not work. To work around those
+    // issues, use sched_setaffinity(0, ...) if available and only otherwise fall back to pthread_setaffinity_np(). See the
+    // following for more information:
+    // - https://github.com/dotnet/runtime/pull/38795
+    // - https://github.com/dotnet/runtime/issues/1634
+    // - https://forum.snapcraft.io/t/requesting-autoconnect-for-interfaces-in-pigmeat-process-control-home/17987/13
+#if HAVE_SCHED_SETAFFINITY
+    int st = sched_setaffinity(0, sizeof(cpu_set_t), &cpuSet);
+#else
     int st = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuSet);
+#endif
 
     return (st == 0);
 
-#else  // HAVE_PTHREAD_GETAFFINITY_NP
+#else  // !(HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP)
     // There is no API to manage thread affinity, so let's ignore the request
     return false;
-#endif // HAVE_PTHREAD_GETAFFINITY_NP
+#endif // HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP
 }
 
 // Boosts the calling thread's thread priority to a level higher than the default

diff --git a/src/coreclr/src/pal/src/config.h.in b/src/coreclr/src/pal/src/config.h.in
@@ -36,7 +36,6 @@
 #cmakedefine01 HAVE_PTHREAD_GETCPUCLOCKID
 #cmakedefine01 HAVE_PTHREAD_SIGQUEUE
 #cmakedefine01 HAVE_PTHREAD_GETAFFINITY_NP
-#cmakedefine01 HAVE_PTHREAD_ATTR_SETAFFINITY_NP
 #cmakedefine01 HAVE_CPUSET_T
 #cmakedefine01 HAVE_SIGRETURN
 #cmakedefine01 HAVE__THREAD_SYS_SIGRETURN
@@ -66,6 +65,7 @@
 #cmakedefine01 HAVE_TTRACE
 #cmakedefine01 HAVE_PIPE2
 #cmakedefine01 HAVE_SCHED_GETAFFINITY
+#cmakedefine01 HAVE_SCHED_SETAFFINITY
 #cmakedefine HAVE_UNW_GET_SAVE_LOC
 #cmakedefine HAVE_UNW_GET_ACCESSORS
 #cmakedefine01 HAVE_XSWDEV

diff --git a/src/coreclr/src/pal/src/configure.cmake b/src/coreclr/src/pal/src/configure.cmake
@@ -81,6 +81,7 @@ check_include_files(gnu/lib-names.h HAVE_GNU_LIBNAMES_H)
 check_function_exists(kqueue HAVE_KQUEUE)
 
 check_library_exists(c sched_getaffinity "" HAVE_SCHED_GETAFFINITY)
+check_library_exists(c sched_setaffinity "" HAVE_SCHED_SETAFFINITY)
 check_library_exists(pthread pthread_create "" HAVE_LIBPTHREAD)
 check_library_exists(c pthread_create "" HAVE_PTHREAD_IN_LIBC)
 
@@ -100,7 +101,6 @@ check_library_exists(${PTHREAD_LIBRARY} pthread_getattr_np "" HAVE_PTHREAD_GETAT
 check_library_exists(${PTHREAD_LIBRARY} pthread_getcpuclockid "" HAVE_PTHREAD_GETCPUCLOCKID)
 check_library_exists(${PTHREAD_LIBRARY} pthread_sigqueue "" HAVE_PTHREAD_SIGQUEUE)
 check_library_exists(${PTHREAD_LIBRARY} pthread_getaffinity_np "" HAVE_PTHREAD_GETAFFINITY_NP)
-check_library_exists(${PTHREAD_LIBRARY} pthread_attr_setaffinity_np "" HAVE_PTHREAD_ATTR_SETAFFINITY_NP)
 
 check_function_exists(sigreturn HAVE_SIGRETURN)
 check_function_exists(_thread_sys_sigreturn HAVE__THREAD_SYS_SIGRETURN)

diff --git a/src/coreclr/src/pal/src/thread/thread.cpp b/src/coreclr/src/pal/src/thread/thread.cpp
@@ -740,41 +740,6 @@ CorUnix::InternalCreateThread(
     storedErrno = errno;
 #endif  // PTHREAD_CREATE_MODIFIES_ERRNO
 
-#if HAVE_PTHREAD_ATTR_SETAFFINITY_NP && HAVE_SCHED_GETAFFINITY
-    {
-        // Threads inherit their parent's affinity mask on Linux. This is not desired, so we reset
-        // the current thread's affinity mask to the mask of the current process.
-        cpu_set_t cpuSet;
-        CPU_ZERO(&cpuSet);
-
-        int st = sched_getaffinity(gPID, sizeof(cpu_set_t), &cpuSet);
-        if (st != 0)
-        {
-            ASSERT("sched_getaffinity failed!\n");
-            // the sched_getaffinity should never fail for getting affinity of the current process
-            palError = ERROR_INTERNAL_ERROR;
-            goto EXIT;
-        }
-
-        st = pthread_attr_setaffinity_np(&pthreadAttr, sizeof(cpu_set_t), &cpuSet);
-        if (st != 0)
-        {
-            if (st == ENOMEM)
-            {
-                palError = ERROR_NOT_ENOUGH_MEMORY;
-            }
-            else
-            {
-                ASSERT("pthread_attr_setaffinity_np failed!\n");
-                // The pthread_attr_setaffinity_np should never fail except of OOM when
-                // passed the mask extracted using sched_getaffinity.
-                palError = ERROR_INTERNAL_ERROR;
-            }
-            goto EXIT;
-        }
-    }
-#endif // HAVE_PTHREAD_GETAFFINITY_NP && HAVE_SCHED_GETAFFINITY
-
     iError = pthread_create(&pthread, &pthreadAttr, CPalThread::ThreadEntry, pNewThread);
 
 #if PTHREAD_CREATE_MODIFIES_ERRNO
@@ -1754,6 +1719,10 @@ CPalThread::ThreadEntry(
     PTHREAD_START_ROUTINE pfnStartRoutine;
     LPVOID pvPar;
     DWORD retValue;
+#if HAVE_SCHED_GETAFFINITY && HAVE_SCHED_SETAFFINITY
+    cpu_set_t cpuSet;
+    int st;
+#endif
 
     pThread = reinterpret_cast<CPalThread*>(pvParam);
 
@@ -1763,6 +1732,42 @@ CPalThread::ThreadEntry(
         goto fail;
     }
 
+#if HAVE_SCHED_GETAFFINITY && HAVE_SCHED_SETAFFINITY
+    // Threads inherit their parent's affinity mask on Linux. This is not desired, so we reset
+    // the current thread's affinity mask to the mask of the current process.
+    //
+    // Typically, we would use pthread_attr_setaffinity_np() and have pthread_create() create the thread with the specified
+    // affinity. At least one implementation of pthread_create() following a pthread_attr_setaffinity_np() calls
+    // sched_setaffinity(<newThreadPid>, ...), which is not allowed under Snap's default strict confinement without manually
+    // connecting the process-control plug. To work around that, have the thread set the affinity after it starts.
+    // sched_setaffinity(<currentThreadPid>, ...) is also currently not allowed, only sched_setaffinity(0, ...).
+    // pthread_setaffinity_np(pthread_self(), ...) seems to call sched_setaffinity(<currentThreadPid>, ...) in at least one
+    // implementation, and does not work. Use sched_setaffinity(0, ...) instead. See the following for more information:
+    // - https://github.com/dotnet/runtime/pull/38795
+    // - https://github.com/dotnet/runtime/issues/1634
+    // - https://forum.snapcraft.io/t/requesting-autoconnect-for-interfaces-in-pigmeat-process-control-home/17987/13
+
+    CPU_ZERO(&cpuSet);
+
+    st = sched_getaffinity(gPID, sizeof(cpu_set_t), &cpuSet);
+    if (st != 0)
+    {
+        ASSERT("sched_getaffinity failed!\n");
+        // The sched_getaffinity should never fail for getting affinity of the current process
+        palError = ERROR_INTERNAL_ERROR;
+        goto fail;
+    }
+
+    st = sched_setaffinity(0, sizeof(cpu_set_t), &cpuSet);
+    if (st != 0)
+    {
+        ASSERT("sched_setaffinity failed!\n");
+        // The sched_setaffinity should never fail when passed the mask extracted using sched_getaffinity
+        palError = ERROR_INTERNAL_ERROR;
+        goto fail;
+    }
+#endif // HAVE_SCHED_GETAFFINITY && HAVE_SCHED_SETAFFINITY
+
 #if !HAVE_MACH_EXCEPTIONS
     if (!pThread->EnsureSignalAlternateStack())
     {
@@ -2946,18 +2951,31 @@ BOOL
 PALAPI
 PAL_SetCurrentThreadAffinity(WORD procNo)
 {
-#if HAVE_PTHREAD_GETAFFINITY_NP
+#if HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP
     cpu_set_t cpuSet;
     CPU_ZERO(&cpuSet);
-
     CPU_SET(procNo, &cpuSet);
+
+    // Snap's default strict confinement does not allow sched_setaffinity(<nonzeroPid>, ...) without manually connecting the
+    // process-control plug. sched_setaffinity(<currentThreadPid>, ...) is also currently not allowed, only
+    // sched_setaffinity(0, ...). pthread_setaffinity_np(pthread_self(), ...) seems to call
+    // sched_setaffinity(<currentThreadPid>, ...) in at least one implementation, and does not work. To work around those
+    // issues, use sched_setaffinity(0, ...) if available and only otherwise fall back to pthread_setaffinity_np(). See the
+    // following for more information:
+    // - https://github.com/dotnet/runtime/pull/38795
+    // - https://github.com/dotnet/runtime/issues/1634
+    // - https://forum.snapcraft.io/t/requesting-autoconnect-for-interfaces-in-pigmeat-process-control-home/17987/13
+#if HAVE_SCHED_SETAFFINITY
+    int st = sched_setaffinity(0, sizeof(cpu_set_t), &cpuSet);
+#else
     int st = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuSet);
+#endif
 
     return st == 0;
-#else  // HAVE_PTHREAD_GETAFFINITY_NP
+#else  // !(HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP)
     // There is no API to manage thread affinity, so let's ignore the request
     return FALSE;
-#endif // HAVE_PTHREAD_GETAFFINITY_NP
+#endif // HAVE_SCHED_SETAFFINITY || HAVE_PTHREAD_SETAFFINITY_NP
 }
 
 /*++