Fix GC tests that contain GC.Collect() used in wrong scope #17680
Conversation
| temp.RunTest(); | ||
|
|
||
| GC.Collect(); | ||
| GC.WaitForPendingFinalizers(); // makes sure Finalize() is called. |
There was a problem hiding this comment.
WaitForPendingFinalizers [](start = 11, length = 24)
might as well add GC.Collect() here to be consistent
| { | ||
| Console.WriteLine("In Finalize of B"); | ||
| } | ||
| Console.WriteLine("In Finalize of B"); } |
There was a problem hiding this comment.
} [](start = 50, length = 2)
not sure why this } is moved... accident?
There was a problem hiding this comment.
Yeah.. Thanks for catching this.
|
so I haven't done an exhaustive look...but based on what I've looked so far it makes more sense to just make this sequence into a little utility function that you call in situations like this: GC.Collect(); so you don't have to worry about adding this to every test that needs it. |
|
By utility function, I assume you mean just for all the GC tests and not like adding a managed API right? |
|
that's correct. it's a test utility function. |
|
Ok, I'll throw that in and change the tests accordingly. |
This same fix has been made in #17656 and #17594. I went through all the GC tests and fixed places where we observe a similar issue. Note that most of these tests don't necessarily depend on this fix, since the JIT won't inline methods / extend lifetime of temps for most of these tests, but wanted to fix these before we see more test failures coming from the various scenarios these tests will be running against.