-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix #84344 to ensure sufficient stack by calling RuntimeHelpers.EnsureSufficientExecutionStack() #122761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix #84344 to ensure sufficient stack by calling RuntimeHelpers.EnsureSufficientExecutionStack() #122761
Conversation
…ient stack space when calling Directory.Delete recursively. It is implemented in both FileSystem.Unix.cs and FileSystem.Windows.cs
1994d04 to
b713486
Compare
|
Rebase as of 29th December, 2025. |
|
Looks like the CI is green (before rebase). I think I will mark this PR as ready (not DRAFT anymore). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #84344 by preventing stack overflow exceptions when recursively deleting deeply nested directory structures. The fix adds stack space validation before making recursive calls in Directory.Delete.
Key Changes
- Added
RuntimeHelpers.EnsureSufficientExecutionStack()calls before recursive directory deletion to prevent stack overflow - Applied consistently to both Windows and Unix filesystem implementations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs | Added using directive and stack validation before recursive RemoveDirectoryRecursive call |
| src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs | Added using directive and stack validation before recursive RemoveDirectoryRecursive call |
…ue, and ensure the nested depth is large enough but not too large as the original scenario (14000). I use 6000 to test whether the stack is still sufficient and run without exception thrown.
|
Done adding unit test for this specific nesting scenario, using 6000 depth nesting instead of original 14000 as the test will run a long time if we use the original 14000 as the depth. @dotnet/area-system-io please review. Let me know if the tag is incorrect so I should tag the right team. |
…the depth to 3000 to avoid System.IO.PathTooLongException on non Windows.
|
Update to limit the depth to only 3000 for non Windows. |
Fix #84344 to ensure sufficient stack by calling
RuntimeHelpers.EnsureSufficientExecutionStack()when callingDirectory.Deleterecursively.It is implemented in both FileSystem.Unix.cs and FileSystem.Windows.cs
I'm creating in draft first, to check the CI.
Looks like this PR needs to have a unit test to cover at least the same scenario as mentioned in the original issue, but unfortunately I could not find the place or file to add a unit test for calling
Directory.Deleterecursively.PS: apologize it takes me more than 3 months to submit this PR, as I finally have been able to test to run locally 🙏