Skip to content

Enlarge stack overflow handling stack #114956

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

janvorli
Copy link
Member

This is an attempt to fix rare failures in the stack overflow tests that I was unable to repro locally. The logs I've added so far seems to indicate that the thread handling the exception crashes with another stack overflow at the point when it waits for a secondary thread to finish dumping stack trace.

This is an attempt to fix rare failures in the stack overflow tests that
I was unable to repro locally. The logs I've added so far seems to indicate
that the thread handling the exception crashes with another stack overflow
at the point when it waits for a secondary thread to finish dumping stack
trace.
@janvorli janvorli added this to the 10.0.0 milestone Apr 23, 2025
@janvorli janvorli requested a review from jkotas April 23, 2025 11:46
@janvorli janvorli self-assigned this Apr 23, 2025
@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 11:46
Copy link
Contributor

@Copilot Copilot AI left a 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 increases the extra stack space allocated for handling stack overflows to mitigate failures observed during testing.

  • Increase the allocated stack size multiplier from 8 to 9 pages.
  • Adjust the stack guard allocation accordingly.

@@ -206,7 +206,7 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags)
}

// Allocate the minimal stack necessary for handling stack overflow
int stackOverflowStackSize = ALIGN_UP(sizeof(SignalHandlerWorkerReturnPoint), 16) + 8 * 4096;
int stackOverflowStackSize = ALIGN_UP(sizeof(SignalHandlerWorkerReturnPoint), 16) + 9 * 4096;
Copy link
Preview

Copilot AI Apr 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an inline comment explaining why the additional 4096 bytes (9 pages) are now required, to aid future maintainers in understanding the rationale behind this change.

Copilot uses AI. Check for mistakes.

@janvorli janvorli merged commit 4309e43 into dotnet:main Apr 23, 2025
94 of 96 checks passed
@janvorli janvorli deleted the enlarge-stack-overflow-stack branch April 23, 2025 17:58
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants