don't aquite lock when calling RuntimeInformation.OSArchitecture and …

…OSArchitecture.ProcessArchitecture (#39196) * don't aquite lock when calling RuntimeInformation.OSArchitecture and OSArchitecture.ProcessArchitecture * make the arch fields volatile * proper fix of the Unix code * apply [SuppressGCTransition]
dotnet · Jul 14, 2020 · 7006ff2 · 7006ff2
1 parent 2c573b5
commit 7006ff2
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 119 deletions.
diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetOSArchitecture.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetOSArchitecture.cs
@@ -8,6 +8,7 @@ internal static partial class Interop
 {
     internal static partial class Sys
     {
+        [SuppressGCTransition]
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetOSArchitecture")]
         internal static extern int GetOSArchitecture();
     }

diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetProcessArchitecture.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetProcessArchitecture.cs
@@ -8,6 +8,7 @@ internal static partial class Interop
 {
     internal static partial class Sys
     {
+        [SuppressGCTransition]
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetProcessArchitecture")]
         internal static extern int GetProcessArchitecture();
     }

diff --git a/...ormation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Unix.cs b/...ormation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Unix.cs
@@ -7,12 +7,8 @@ namespace System.Runtime.InteropServices
 {
     public static partial class RuntimeInformation
     {
-        private static readonly object s_osLock = new object();
-        private static readonly object s_processLock = new object();
         private static string? s_osPlatformName;
         private static string? s_osDescription;
-        private static Architecture? s_osArch;
-        private static Architecture? s_processArch;
 
         internal static bool IsCurrentOSPlatform(string osPlatform)
         {
@@ -24,73 +20,24 @@ internal static bool IsCurrentOSPlatform(string osPlatform)
 
         public static string OSDescription => s_osDescription ??= Interop.Sys.GetUnixVersion();
 
-        public static Architecture OSArchitecture
-        {
-            get
-            {
-                lock (s_osLock)
-                {
-                    if (null == s_osArch)
-                    {
-                        Interop.Sys.ProcessorArchitecture arch = (Interop.Sys.ProcessorArchitecture)Interop.Sys.GetOSArchitecture();
-                        switch (arch)
-                        {
-                            case Interop.Sys.ProcessorArchitecture.ARM:
-                                s_osArch = Architecture.Arm;
-                                break;
-
-                            case Interop.Sys.ProcessorArchitecture.x64:
-                                s_osArch = Architecture.X64;
-                                break;
+        public static Architecture OSArchitecture { get; } = Map((Interop.Sys.ProcessorArchitecture)Interop.Sys.GetOSArchitecture());
 
-                            case Interop.Sys.ProcessorArchitecture.x86:
-                                s_osArch = Architecture.X86;
-                                break;
+        public static Architecture ProcessArchitecture { get; } = Map((Interop.Sys.ProcessorArchitecture)Interop.Sys.GetProcessArchitecture());
 
-                            case Interop.Sys.ProcessorArchitecture.ARM64:
-                                s_osArch = Architecture.Arm64;
-                                break;
-                        }
-                    }
-                }
-
-                Debug.Assert(s_osArch != null);
-                return s_osArch.Value;
-            }
-        }
-
-        public static Architecture ProcessArchitecture
+        private static Architecture Map(Interop.Sys.ProcessorArchitecture arch)
         {
-            get
+            switch (arch)
             {
-                lock (s_processLock)
-                {
-                    if (null == s_processArch)
-                    {
-                        Interop.Sys.ProcessorArchitecture arch = (Interop.Sys.ProcessorArchitecture)Interop.Sys.GetProcessArchitecture();
-                        switch (arch)
-                        {
-                            case Interop.Sys.ProcessorArchitecture.ARM:
-                                s_processArch = Architecture.Arm;
-                                break;
-
-                            case Interop.Sys.ProcessorArchitecture.x64:
-                                s_processArch = Architecture.X64;
-                                break;
-
-                            case Interop.Sys.ProcessorArchitecture.x86:
-                                s_processArch = Architecture.X86;
-                                break;
-
-                            case Interop.Sys.ProcessorArchitecture.ARM64:
-                                s_processArch = Architecture.Arm64;
-                                break;
-                        }
-                    }
-                }
-
-                Debug.Assert(s_processArch != null);
-                return s_processArch.Value;
+                case Interop.Sys.ProcessorArchitecture.ARM:
+                    return Architecture.Arm;
+                case Interop.Sys.ProcessorArchitecture.x64:
+                    return Architecture.X64;
+                case Interop.Sys.ProcessorArchitecture.ARM64:
+                    return Architecture.Arm64;
+                case Interop.Sys.ProcessorArchitecture.x86:
+                default:
+                    Debug.Assert(arch == Interop.Sys.ProcessorArchitecture.x86, "Unidentified Architecture");
+                    return Architecture.X86;
             }
         }
     }

diff --git a/...ation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Windows.cs b/...ation/src/System/Runtime/InteropServices/RuntimeInformation/RuntimeInformation.Windows.cs
@@ -8,10 +8,8 @@ namespace System.Runtime.InteropServices
     public static partial class RuntimeInformation
     {
         private static string? s_osDescription;
-        private static readonly object s_osLock = new object();
-        private static readonly object s_processLock = new object();
-        private static Architecture? s_osArch;
-        private static Architecture? s_processArch;
+        private static volatile int s_osArch = -1;
+        private static volatile int s_processArch = -1;
 
         internal static bool IsCurrentOSPlatform(string osPlatform) => osPlatform.Equals("WINDOWS", StringComparison.OrdinalIgnoreCase);
 
@@ -39,68 +37,52 @@ public static Architecture OSArchitecture
         {
             get
             {
-                lock (s_osLock)
-                {
-                    if (null == s_osArch)
-                    {
-                        Interop.Kernel32.SYSTEM_INFO sysInfo;
-                        Interop.Kernel32.GetNativeSystemInfo(out sysInfo);
+                Debug.Assert(sizeof(Architecture) == sizeof(int));
 
-                        switch ((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture)
-                        {
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM64:
-                                s_osArch = Architecture.Arm64;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM:
-                                s_osArch = Architecture.Arm;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_AMD64:
-                                s_osArch = Architecture.X64;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_INTEL:
-                                s_osArch = Architecture.X86;
-                                break;
-                        }
+                int osArch = s_osArch;
 
-                    }
+                if (osArch == -1)
+                {
+                    Interop.Kernel32.GetNativeSystemInfo(out Interop.Kernel32.SYSTEM_INFO sysInfo);
+                    osArch = s_osArch = (int)Map((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture);
                 }
 
-                Debug.Assert(s_osArch != null);
-                return s_osArch.Value;
+                return (Architecture)osArch;
             }
         }
 
         public static Architecture ProcessArchitecture
         {
             get
             {
-                lock (s_processLock)
-                {
-                    if (null == s_processArch)
-                    {
-                        Interop.Kernel32.SYSTEM_INFO sysInfo;
-                        Interop.Kernel32.GetSystemInfo(out sysInfo);
+                Debug.Assert(sizeof(Architecture) == sizeof(int));
 
-                        switch ((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture)
-                        {
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM64:
-                                s_processArch = Architecture.Arm64;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM:
-                                s_processArch = Architecture.Arm;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_AMD64:
-                                s_processArch = Architecture.X64;
-                                break;
-                            case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_INTEL:
-                                s_processArch = Architecture.X86;
-                                break;
-                        }
-                    }
+                int processArch = s_processArch;
+
+                if (processArch == -1)
+                {
+                    Interop.Kernel32.GetSystemInfo(out Interop.Kernel32.SYSTEM_INFO sysInfo);
+                    processArch = s_processArch = (int)Map((Interop.Kernel32.ProcessorArchitecture)sysInfo.wProcessorArchitecture);
                 }
 
-                Debug.Assert(s_processArch != null);
-                return s_processArch.Value;
+                return (Architecture)processArch;
+            }
+        }
+
+        private static Architecture Map(Interop.Kernel32.ProcessorArchitecture processorArchitecture)
+        {
+            switch (processorArchitecture)
+            {
+                case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM64:
+                    return Architecture.Arm64;
+                case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_ARM:
+                    return Architecture.Arm;
+                case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_AMD64:
+                    return Architecture.X64;
+                case Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_INTEL:
+                default:
+                    Debug.Assert(processorArchitecture == Interop.Kernel32.ProcessorArchitecture.Processor_Architecture_INTEL, "Unidentified Architecture");
+                    return Architecture.X86;
             }
         }
     }