Merge pull request #2185 from stephentoub/process_osx_fix

Fix Process bugs on OSX

Author: stephentoub

src/Common/src/Interop/OSX/Interop.libproc.cs

@@ -25,7 +25,7 @@ internal static partial class libproc
         private const int PROC_PIDPATHINFO_MAXSIZE = 4 * MAXPATHLEN;
         private static int PROC_PIDLISTFD_SIZE = Marshal.SizeOf<proc_fdinfo>();
         private static int PROC_PIDLISTTHREADS_SIZE = (Marshal.SizeOf<uint>() * 2);
-        
+
         // Constants from sys\resource.h
         private const int RUSAGE_SELF = 0;
 
@@ -364,6 +364,7 @@ private static unsafe extern int proc_pidinfo(
             int result = 0;
             int size = 20; // start assuming 20 threads is enough
             ulong[] threadIds = null;
+            var threads = new List<KeyValuePair<ulong, proc_threadinfo?>>();
 
             // We have no way of knowning how many threads the process has (and therefore how big our buffer should be)
             // so while the return value of the function is the same as our buffer size (meaning it completely filled
@@ -378,7 +379,10 @@ private static unsafe extern int proc_pidinfo(
 
                 if (result <= 0)
                 {
-                    throw new Win32Exception();
+                    // If we were unable to access the information, just return the empty list.  
+                    // This is likely to happen for privileged processes, if the process went away
+                    // by the time we tried to query it, etc.
+                    return threads;
                 }
                 else
                 {
@@ -394,7 +398,7 @@ private static unsafe extern int proc_pidinfo(
 
             // Loop over each thread and get the thread info
             int count = (int)(result / Marshal.SizeOf<ulong>());
-            List<KeyValuePair<ulong, proc_threadinfo?>> threads = new List<KeyValuePair<ulong, proc_threadinfo?>>(count);
+            threads.Capacity = count;
             for (int i = 0; i < count; i++)
             {        
                 threads.Add(new KeyValuePair<ulong, proc_threadinfo?>(threadIds[i], GetThreadInfoById(pid, threadIds[i])));
@@ -406,7 +410,7 @@ private static unsafe extern int proc_pidinfo(
         /// <summary>
         /// Retrieves the number of open file descriptors for the specified pid
         /// </summary>
-        /// <returns>Returns an array of open File Descriptors for this process</returns>
+        /// <returns>A count of file descriptors for this process.</returns>
         /// <remarks>
         /// This function doesn't use the helper since it seems to allow passing NULL
         /// values in to the buffer and length parameters to get back an estimation 
@@ -426,11 +430,14 @@ internal static unsafe int GetFileDescriptorCountForPid(int pid)
             int result = proc_pidinfo(pid, PROC_PIDLISTFDS, 0, (proc_fdinfo*)null, 0);
             if (result <= 0)
             {
-                throw new Win32Exception();
+                // If we were unable to access the information, just return the empty list.  
+                // This is likely to happen for privileged processes, if the process went away
+                // by the time we tried to query it, etc.
+                return 0;
             }
 
             proc_fdinfo[] fds;
-            int size = (int)(result / Marshal.SizeOf<proc_fdinfo>());
+            int size = (int)(result / Marshal.SizeOf<proc_fdinfo>()) + 1;
 
             // Just in case the app opened a ton of handles between when we asked and now,
             // make sure we retry if our buffer is filled
@@ -444,7 +451,10 @@ internal static unsafe int GetFileDescriptorCountForPid(int pid)
 
                 if (result <= 0)
                 {
-                    throw new Win32Exception();
+                    // If we were unable to access the information, just return the empty list.  
+                    // This is likely to happen for privileged processes, if the process went away
+                    // by the time we tried to query it, etc.
+                    return 0;
                 }
                 else
                 {

src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.OSX.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Generic;
+using System.Runtime.InteropServices;
 
 namespace System.Diagnostics
 {
@@ -17,33 +18,29 @@ public static int[] GetProcessIds()
         // ---- PAL layer ends here ----
         // -----------------------------
 
-        private unsafe static ProcessInfo CreateProcessInfo(int pid)
+        private static ProcessInfo CreateProcessInfo(int pid)
         {
             // Negative PIDs aren't valid
             if (pid < 0)
             {
                 throw new ArgumentOutOfRangeException("pid");
             }
 
-            ProcessInfo procInfo = new ProcessInfo();
+            ProcessInfo procInfo = new ProcessInfo()
+            {
+                ProcessId = pid
+            };
 
             // Try to get the task info. This can fail if the user permissions don't permit
             // this user context to query the specified process
             Interop.libproc.proc_taskallinfo? info = Interop.libproc.GetProcessInfoById(pid);
             if (info.HasValue)
             {
-                // We need to convert the byte pointer to an IntPtr 
-                // that we can pass to the Marshal.PtrToStringAnsi call
-                // but the nullable struct type makes it difficult to inline,
-                // so make a temp variable to remove the nullable and get the pointer 
-                Interop.libproc.proc_taskallinfo temp = info.Value;
-                IntPtr ptrString = new IntPtr(temp.pbsd.pbi_comm);
-
                 // Set the values we have; all the other values don't have meaning or don't exist on OSX
+                Interop.libproc.proc_taskallinfo temp = info.Value;
+                unsafe { procInfo.ProcessName = Marshal.PtrToStringAnsi(new IntPtr(temp.pbsd.pbi_comm)); }
                 procInfo.BasePriority = temp.ptinfo.pti_priority;
                 procInfo.HandleCount = Interop.libproc.GetFileDescriptorCountForPid(pid);
-                procInfo.ProcessId = pid;
-                procInfo.ProcessName = System.Runtime.InteropServices.Marshal.PtrToStringAnsi(ptrString);
                 procInfo.VirtualBytes = (long)temp.ptinfo.pti_virtual_size;
                 procInfo.WorkingSet = (long)temp.ptinfo.pti_resident_size;
             }
@@ -52,19 +49,23 @@ private unsafe static ProcessInfo CreateProcessInfo(int pid)
             List<KeyValuePair<ulong, Interop.libproc.proc_threadinfo?>> lstThreads = Interop.libproc.GetAllThreadsInProcess(pid);
             foreach (KeyValuePair<ulong, Interop.libproc.proc_threadinfo?> t in lstThreads)
             {
+                var ti = new ThreadInfo()
+                {
+                    _processId = pid,
+                    _threadId = (int)t.Key, // The OS X thread ID is 64-bits, but we're forced to truncate due to the public API signature
+                    _basePriority = 0,
+                    _startAddress = IntPtr.Zero
+                };
+
+                // Fill in additional info if we were able to retrieve such data about the thread
                 if (t.Value.HasValue)
                 {
-                    procInfo._threadInfoList.Add(new ThreadInfo()
-                    {
-                        _basePriority = 0,
-                        _currentPriority = t.Value.Value.pth_curpri,
-                        _processId = pid,
-                        _startAddress = IntPtr.Zero, // We don't have this info
-                        _threadId = Convert.ToInt32(t.Key),
-                        _threadState = ConvertOsxThreadRunStateToThreadState((Interop.libproc.ThreadRunState)t.Value.Value.pth_run_state),
-                        _threadWaitReason = ConvertOsxThreadFlagsToWaitReason((Interop.libproc.ThreadFlags)t.Value.Value.pth_flags)
-                    });
+                    ti._currentPriority = t.Value.Value.pth_curpri;
+                    ti._threadState = ConvertOsxThreadRunStateToThreadState((Interop.libproc.ThreadRunState)t.Value.Value.pth_run_state);
+                    ti._threadWaitReason = ConvertOsxThreadFlagsToWaitReason((Interop.libproc.ThreadFlags)t.Value.Value.pth_flags);
                 }
+
+                procInfo._threadInfoList.Add(ti);
             }
 
             return procInfo;

src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests/ProcessTest.cs

@@ -79,7 +79,7 @@ public void SetAndCheckBasePriority(ProcessPriorityClass exPriorityClass, int pr
             Assert.Equal(priority, _process.BasePriority);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_BasePriority()
         {
             ProcessPriorityClass originalPriority = _process.PriorityClass;
@@ -119,7 +119,7 @@ public void StartAndKillProcessWithDelay(Process p)
             Assert.True(p.WaitForExit(WaitInMS));
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_EnableRaiseEvents()
         {
             {
@@ -156,7 +156,7 @@ public void Process_EnableRaiseEvents()
             }
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_ExitCode()
         {
             {
@@ -261,11 +261,17 @@ public void Process_MainModule()
             }
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_MaxWorkingSet()
         {
-            IntPtr min, max;
-            uint flags;
+            using (Process p = Process.GetCurrentProcess())
+            {
+                Assert.True((long)p.MaxWorkingSet > 0);
+                Assert.True((long)p.MinWorkingSet >= 0);
+            }
+
+            if (global::Interop.IsOSX)
+                return; // doesn't support getting/setting working set for other processes
 
             long curValue = (long)_process.MaxWorkingSet;
             Assert.True(curValue >= 0);
@@ -275,6 +281,9 @@ public void Process_MaxWorkingSet()
                 try
                 {
                     _process.MaxWorkingSet = (IntPtr)((int)curValue + 1024);
+
+                    IntPtr min, max;
+                    uint flags;
                     Interop.GetProcessWorkingSetSizeEx(_process.SafeHandle, out min, out max, out flags);
                     curValue = (int)max;
                     _process.Refresh();
@@ -287,9 +296,18 @@ public void Process_MaxWorkingSet()
             }
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_MinWorkingSet()
         {
+            using (Process p = Process.GetCurrentProcess())
+            {
+                Assert.True((long)p.MaxWorkingSet > 0);
+                Assert.True((long)p.MinWorkingSet >= 0);
+            }
+
+            if (global::Interop.IsOSX)
+                return; // doesn't support getting/setting working set for other processes
+
             long curValue = (long)_process.MinWorkingSet;
             Assert.True(curValue >= 0);
 
@@ -341,57 +359,59 @@ private void AssertNonZeroWindowsZeroUnix(long value)
             }
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_NonpagedSystemMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.NonpagedSystemMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PagedMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PagedMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PagedSystemMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PagedSystemMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PeakPagedMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PeakPagedMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PeakVirtualMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PeakVirtualMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PeakWorkingSet64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PeakWorkingSet64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_PrivateMemorySize64()
         {
             AssertNonZeroWindowsZeroUnix(_process.PrivateMemorySize64);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
-        public void Process_PrivilegedProcessorTime()
+        [Fact]
+        [ActiveIssue(2184, PlatformID.OSX)]
+        public void Process_ProcessorTime()
         {
             Assert.True(_process.UserProcessorTime.TotalSeconds >= 0);
             Assert.True(_process.PrivilegedProcessorTime.TotalSeconds >= 0);
             Assert.True(_process.TotalProcessorTime.TotalSeconds >= 0);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
+        [PlatformSpecific(~PlatformID.OSX)] // getting/setting affinity not supported on OSX
         public void Process_ProcessorAffinity()
         {
             IntPtr curProcessorAffinity = _process.ProcessorAffinity;
@@ -449,7 +469,7 @@ public void Process_InvalidPriorityClass()
             Assert.Throws<ArgumentException>(() => { p.PriorityClass = ProcessPriorityClass.Normal | ProcessPriorityClass.Idle; });
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void ProcessProcessName()
         {
             Assert.Equal(_process.ProcessName, CoreRunName, StringComparer.OrdinalIgnoreCase);
@@ -462,7 +482,7 @@ public void ProcessProcessName()
         [DllImport("libc")]
         internal static extern int getpid();
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_GetCurrentProcess()
         {
             Process current = Process.GetCurrentProcess();
@@ -476,7 +496,7 @@ public void Process_GetCurrentProcess()
             Assert.Equal(Process.GetProcessById(currentProcessId).ProcessName, Process.GetCurrentProcess().ProcessName);
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_GetProcesses()
         {
             // Get all the processes running on the machine.
@@ -494,7 +514,7 @@ public void Process_GetProcesses()
             Assert.True(foundCurrentProcess, "Process_GetProcesses002 failed");
         }
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_GetProcessesByName()
         {
             // Get the current process using its name
@@ -760,5 +780,25 @@ public void Process_IPC()
             }
         }
 
+        [Fact]
+        public void ThreadCount()
+        {
+            Assert.True(_process.Threads.Count > 0);
+            using (Process p = Process.GetCurrentProcess())
+            {
+                Assert.True(p.Threads.Count > 0);
+            }
+        }
+
+        // [Fact] // uncomment for diagnostic purposes to list processes to console
+        public void ConsoleWriteLineProcesses()
+        {
+            foreach (var p in Process.GetProcesses().OrderBy(p => p.Id))
+            {
+                Console.WriteLine("{0} : \"{1}\" (Threads: {2})", p.Id, p.ProcessName, p.Threads.Count);
+                p.Dispose();
+            }
+        }
+
     }
 }

src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests/Process_EncodingTest.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Text;
-using System.Diagnostics;
 using Xunit;
 
 namespace System.Diagnostics.ProcessTests
@@ -23,7 +22,7 @@ public partial class ProcessTest
 
         private const int s_ConsoleEncoding = 437;
 
-        [Fact, ActiveIssue(1538, PlatformID.OSX)]
+        [Fact]
         public void Process_EncodingBeforeProvider()
         {
             Action<int> run = expectedCodePage =>