Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException

Fixes issue #105422 On MacOS, FreeBSD, SunOS (pretty much any port using ProcessBSD.c) the get/set WorkingSet methods only work on the current process. Skip the parts of tests that operate on other processes. There are tests that verify the old (less correct) behavior of the get/set WorkingSet methods from before this fix. Comment out those tests for now and add notes. Needs testing on the affected platforms and then either update those commented out tests or just remove them.
dotnet · Jul 27, 2024 · 5b1ce56 · 5b1ce56
1 parent ceccfe0
commit 5b1ce56
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 17 deletions.
diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.BSD.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.BSD.cs
@@ -57,6 +57,8 @@ private static IntPtr ProcessorAffinityCore
         /// </summary>
         private void GetWorkingSetLimits(out IntPtr minWorkingSet, out IntPtr maxWorkingSet)
         {
+            EnsureState(State.HaveNonExitedId);
+
             // We can only do this for the current process on OS X
             if (_processId != Environment.ProcessId)
                 throw new PlatformNotSupportedException(SR.OsxExternalProcessWorkingSetNotSupported);
@@ -86,6 +88,8 @@ private void GetWorkingSetLimits(out IntPtr minWorkingSet, out IntPtr maxWorking
         /// <param name="resultingMax">The resulting maximum working set limit after any changes applied.</param>
         private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr resultingMin, out IntPtr resultingMax)
         {
+            EnsureState(State.HaveNonExitedId);
+
             // We can only do this for the current process on OS X
             if (_processId != Environment.ProcessId)
                 throw new PlatformNotSupportedException(SR.OsxExternalProcessWorkingSetNotSupported);

diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
@@ -601,22 +601,26 @@ public void TestMaxWorkingSet()
             }
         }
 
+        // XXX No longer skip: OSX, FreeBSD
         [Fact]
-        [SkipOnPlatform(TestPlatforms.OSX | TestPlatforms.FreeBSD | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Getting MaxWorkingSet is not supported on OSX, BSD, iOS, MacCatalyst, and tvOS.")]
+        [SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Getting MaxWorkingSet is not supported on OSX, BSD, iOS, MacCatalyst, and tvOS.")]
         public void MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException()
         {
             var process = new Process();
             Assert.Throws<InvalidOperationException>(() => process.MaxWorkingSet);
         }
 
-        [Fact]
-        [PlatformSpecific(TestPlatforms.OSX | TestPlatforms.FreeBSD)]
-        public void MaxValueWorkingSet_GetSetMacos_ThrowsPlatformSupportedException()
-        {
-            var process = new Process();
-            Assert.Throws<PlatformNotSupportedException>(() => process.MaxWorkingSet);
-            Assert.Throws<PlatformNotSupportedException>(() => process.MaxWorkingSet = (IntPtr)1);
-        }
+        // XXX This appears to be testing for incorrect behavoir which has been fixed.
+        // XXX See https://github.com/dotnet/runtime/issues/105422
+        // The correct exception for a new process is: InvalidOperationException
+        // [Fact]
+        // [PlatformSpecific(TestPlatforms.OSX | TestPlatforms.FreeBSD)]
+        // public void MaxValueWorkingSet_GetSetMacos_ThrowsPlatformSupportedException()
+        // {
+        //    var process = new Process();
+        //    Assert.Throws<PlatformNotSupportedException>(() => process.MaxWorkingSet);
+        //    Assert.Throws<PlatformNotSupportedException>(() => process.MaxWorkingSet = (IntPtr)1);
+        // }
 
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public void TestMinWorkingSet()
@@ -656,21 +660,25 @@ public void TestMinWorkingSet()
             }
         }
 
+        // XXX No longer skip: OSX, FreeBSD
         [Fact]
-        [SkipOnPlatform(TestPlatforms.OSX | TestPlatforms.FreeBSD | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Getting MinWorkingSet is not supported on OSX, BSD, iOS, MacCatalyst, and tvOS.")]
+        [SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Getting MinWorkingSet is not supported on OSX, BSD, iOS, MacCatalyst, and tvOS.")]
         public void MinWorkingSet_GetNotStarted_ThrowsInvalidOperationException()
         {
             var process = new Process();
             Assert.Throws<InvalidOperationException>(() => process.MinWorkingSet);
         }
 
-        [Fact]
-        [PlatformSpecific(TestPlatforms.OSX | TestPlatforms.FreeBSD)]
-        public void MinWorkingSet_GetMacos_ThrowsPlatformSupportedException()
-        {
-            var process = new Process();
-            Assert.Throws<PlatformNotSupportedException>(() => process.MinWorkingSet);
-        }
+        // XXX This appears to be testing for incorrect behavoir which has been fixed.
+        // XXX See https://github.com/dotnet/runtime/issues/105422
+        // The correct exception for a new process is: InvalidOperationException
+        // [Fact]
+        // [PlatformSpecific(TestPlatforms.OSX | TestPlatforms.FreeBSD)]
+        // public void MinWorkingSet_GetMacos_ThrowsPlatformSupportedException()
+        // {
+        //    var process = new Process();
+        //    Assert.Throws<PlatformNotSupportedException>(() => process.MinWorkingSet);
+        // }
 
         [Fact]
         public void TestModules()