Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException #105424

gwr · 2024-07-24T19:46:23Z

stephentoub

Thanks

stephentoub · 2024-07-25T04:19:40Z

These tests needs to be addressed:

runtime/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs

Lines 659 to 673 in 54a9efd

    
           [Fact] 
        
           [SkipOnPlatform(TestPlatforms.OSX | TestPlatforms.FreeBSD | 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); 
        
           }

gwr · 2024-07-25T12:58:41Z

These tests needs to be addressed:

runtime/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs

Lines 659 to 673 in 54a9efd

[Fact]

[SkipOnPlatform(TestPlatforms.OSX | TestPlatforms.FreeBSD | 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);

}

Yeah, the SkipOnPlatform line(s) probably need adjustment. I think these tests should now pass on FreeBSD and OSX.
Someone needs to find time to remove the OS "skip" for each of these and try the test with that fix.

am11 · 2024-07-25T13:09:39Z

In CI, this test is failing on osx with your changes:

    System.Diagnostics.Tests.ProcessTests.MinWorkingSet_GetMacos_ThrowsPlatformSupportedException [FAIL]
      Assert.Throws() Failure: Exception type was not an exact match
      Expected: typeof(System.PlatformNotSupportedException)
      Actual:   typeof(System.InvalidOperationException)
      ---- System.InvalidOperationException : No process is associated with this object.
      Stack Trace:
        /_/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs(672,0): at System.Diagnostics.Tests.ProcessTests.MinWorkingSet_GetMacos_ThrowsPlatformSupportedException()
        
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(50,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
        ----- Inner Stack Trace -----
        /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs(949,0): at System.Diagnostics.Process.EnsureState(State state)
        /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs(961,0): at System.Diagnostics.Process.EnsureState(State state)
        /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.BSD.cs(60,0): at System.Diagnostics.Process.GetWorkingSetLimits(IntPtr& minWorkingSet, IntPtr& maxWorkingSet)
        /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs(1014,0): at System.Diagnostics.Process.EnsureWorkingSetLimits()
        /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs(295,0): at System.Diagnostics.Process.get_MinWorkingSet()
        /_/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs(672,0): at System.Diagnostics.Tests.ProcessTests.<>c__DisplayClass40_0.<MinWorkingSet_GetMacos_ThrowsPlatformSupportedException>b__0()
    System.Diagnostics.Tests.ProcessTests.TestProcessRecycledPid [SKIP]

https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-105424-merge-dd30dcce610b4a389e/System.Diagnostics.Process.Tests/1/console.4d1998ff.log?helixlogtype=result

am11 · 2024-07-25T13:18:42Z

Yeah, the SkipOnPlatform line(s) probably need adjustment. I think these tests should now pass on FreeBSD and OSX.

Feel free to merge both tests into one and delete the [Skip line here.

@Thefrank, a heads-up for the next test run on FreeBSD after this PR is merged.

gwr · 2024-07-25T14:15:02Z

In CI, this test is failing on osx with your changes:
    System.Diagnostics.Tests.ProcessTests.MinWorkingSet_GetMacos_ThrowsPlatformSupportedException [FAIL]
    System.Diagnostics.Tests.ProcessTests.TestProcessRecycledPid [SKIP]
https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-105424-merge-dd30dcce610b4a389e/System.Diagnostics.Process.Tests/1/console.4d1998ff.log?helixlogtype=result

Thanks. Clearly the fix in Process.BSD.cs needs testing on OSX and FreBSD.
I've taken a stab at what I think will be needed to fix it, pushed to the PR.

gwr · 2024-07-25T16:23:33Z

src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs

+	// 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
+#if 0


Is this #if 0 not allowed? I don't mean for it to stay long term, just until testing is complete.

We wouldn't merge this, if that's what you mean. If you want to experiment with it in CI, that's fine. I'm going to move the PR to be draft until it's ready.

OK, though I was hoping the CI stuff could help me with some of this, and now it seems that doesn't happen.

CI still runs on drafts

Ah, OK. I guess I misunderstood what I saw on the web page. Thx.

gwr · 2024-07-27T18:11:36Z

I reorganized the changes related to this so it can go after #105403

gwr · 2024-08-18T15:42:45Z

Rebased onto main

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.BSD.cs

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs

Fixes issue dotnet#105422 On MacOS, FreeBSD, SunOS (the ports that share ProcessBSD.c) the get/set WorkingSet methods only work on the current process. Skip the parts of tests that operate on other processes. Remove now redundant MacOS-speicifc tests.

am11 · 2024-08-22T16:15:22Z

@stephentoub please give it another pass. :) (the temporary blocks were removed)

gwr · 2024-08-22T19:38:38Z

Can anyone tell me what to do about the two runtime checks that reported failure?

gwr · 2024-08-28T17:34:48Z

nudge 2

gwr · 2024-09-12T13:16:21Z

nudge 3

am11 · 2024-09-12T17:25:42Z

Can anyone tell me what to do about the two runtime checks that reported failure?

When Build Analysis leg is green, that means the remaining ones are known failures unrelated to your changes.

am11 · 2024-10-21T08:11:34Z

@marek-safar it applies to both of those operating systems.

danmoseley · 2024-12-02T20:33:46Z

@stephentoub this seems good to go?

gwr · 2024-12-19T17:54:36Z

Nudge...

dotnet-issue-labeler bot added the area-System.Diagnostics.Process label Jul 24, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 24, 2024

gwr mentioned this pull request Jul 24, 2024

SunOS process and thread support #105403

Open

am11 added os-freebsd FreeBSD OS os-mac-os-x macOS aka OSX labels Jul 24, 2024

This was referenced Jul 24, 2024

SIGKILL (OOM?) while running LibraryImportGenerator.Tests w/o actionable log messages or artifacts dotnet/dnceng#2496

Open

ExplicitConversion_FromSingle failing due to NaN != NaN #103347

Open

TimeProviderTests.TestProviderTimer failed in CI #103459

Closed

stephentoub approved these changes Jul 25, 2024

View reviewed changes

stephentoub self-requested a review July 25, 2024 04:20

gwr force-pushed the bsd-ws-limits branch from f46bebc to f064815 Compare July 25, 2024 15:23

gwr commented Jul 25, 2024

View reviewed changes

gwr force-pushed the bsd-ws-limits branch from f064815 to 33b69d0 Compare July 25, 2024 16:25

stephentoub marked this pull request as draft July 25, 2024 16:28

gwr force-pushed the bsd-ws-limits branch 2 times, most recently from 2228476 to 6825a06 Compare July 27, 2024 18:09

gwr force-pushed the bsd-ws-limits branch from 6825a06 to 5b1ce56 Compare July 27, 2024 18:25

gwr force-pushed the bsd-ws-limits branch 2 times, most recently from cf89c00 to d6d218d Compare July 29, 2024 19:46

gwr marked this pull request as ready for review July 29, 2024 19:49

build-analysis bot mentioned this pull request Jul 29, 2024

[browser] RuntimeError: memory access out of bounds #96546

Closed

gwr mentioned this pull request Aug 16, 2024

Tracking issue for illumos and Solaris x86-64 port work #34944

Open

19 tasks

gwr force-pushed the bsd-ws-limits branch from d6d218d to 328af4c Compare August 18, 2024 15:41

gwr requested a review from am11 August 18, 2024 15:43

build-analysis bot mentioned this pull request Aug 18, 2024

Test failure "RemoteExecutionException : Half-way through waiting for remote process." in System.Threading.ThreadPools.Tests.ThreadPoolTests.IOCompletionPortCountConfigVarTest #106494

Closed

am11 reviewed Aug 21, 2024

View reviewed changes

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.BSD.cs Show resolved Hide resolved

gwr force-pushed the bsd-ws-limits branch 2 times, most recently from 311df71 to f72eec3 Compare August 21, 2024 22:01

gwr requested a review from am11 August 21, 2024 22:02

am11 reviewed Aug 21, 2024

View reviewed changes

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs Outdated Show resolved Hide resolved

gwr force-pushed the bsd-ws-limits branch from f72eec3 to 591973d Compare August 21, 2024 23:08

am11 approved these changes Aug 22, 2024

View reviewed changes

gwr force-pushed the bsd-ws-limits branch from 591973d to 10dfd9b Compare August 22, 2024 16:11

marek-safar added os-freebsd FreeBSD OS and removed os-freebsd FreeBSD OS os-mac-os-x macOS aka OSX labels Oct 21, 2024

am11 added the os-mac-os-x macOS aka OSX label Oct 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException #105424

Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException #105424

gwr commented Jul 24, 2024

stephentoub left a comment

stephentoub commented Jul 25, 2024

gwr commented Jul 25, 2024

am11 commented Jul 25, 2024

am11 commented Jul 25, 2024

gwr commented Jul 25, 2024

gwr Jul 25, 2024

stephentoub Jul 25, 2024

gwr Jul 25, 2024

stephentoub Jul 25, 2024

gwr Jul 25, 2024

gwr commented Jul 27, 2024

gwr commented Aug 18, 2024

am11 commented Aug 22, 2024

gwr commented Aug 22, 2024

gwr commented Aug 28, 2024 •

edited

Loading

gwr commented Sep 12, 2024

am11 commented Sep 12, 2024

am11 commented Oct 21, 2024

danmoseley commented Dec 2, 2024

gwr commented Dec 19, 2024

Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException #105424

Are you sure you want to change the base?

Fix MaxWorkingSet_GetNotStarted_ThrowsInvalidOperationException #105424

Conversation

gwr commented Jul 24, 2024

stephentoub left a comment

Choose a reason for hiding this comment

stephentoub commented Jul 25, 2024

gwr commented Jul 25, 2024

am11 commented Jul 25, 2024

am11 commented Jul 25, 2024

gwr commented Jul 25, 2024

gwr Jul 25, 2024

Choose a reason for hiding this comment

stephentoub Jul 25, 2024

Choose a reason for hiding this comment

gwr Jul 25, 2024

Choose a reason for hiding this comment

stephentoub Jul 25, 2024

Choose a reason for hiding this comment

gwr Jul 25, 2024

Choose a reason for hiding this comment

gwr commented Jul 27, 2024

gwr commented Aug 18, 2024

am11 commented Aug 22, 2024

gwr commented Aug 22, 2024

gwr commented Aug 28, 2024 • edited Loading

gwr commented Sep 12, 2024

am11 commented Sep 12, 2024

am11 commented Oct 21, 2024

danmoseley commented Dec 2, 2024

gwr commented Dec 19, 2024

gwr commented Aug 28, 2024 •

edited

Loading