Skip to content
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

Update DeallocationStack for Windows context switch #15032

Merged

Conversation

HertzDevil
Copy link
Contributor

Calling LibC.GetCurrentThreadStackLimits outside the main fiber of a thread will continue to return the original stack top of that thread:

LibC.GetCurrentThreadStackLimits(out low_limit, out high_limit)
low_limit  # => 86767566848
high_limit # => 86775955456

spawn do
  LibC.GetCurrentThreadStackLimits(out low_limit2, out high_limit2)
  low_limit2  # => 86767566848
  high_limit2 # => 1863570554880
end

sleep 1.second

This doesn't affect the Crystal runtime because the function is only called once to initialize the main thread, nor the GC because it probes the stack top using VirtualQuery. It may nonetheless affect other external libraries because the bad stack will most certainly contain unmapped memory.

The correct way is to also update the Win32-specific, non-NT DeallocationStack field, since this is the actual stack top that gets returned (see also Wine's implementation).

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:concurrency platform:windows labels Sep 25, 2024
@straight-shoota straight-shoota added this to the 1.14.0 milestone Sep 25, 2024
@straight-shoota straight-shoota merged commit 48883e2 into crystal-lang:master Sep 26, 2024
65 checks passed
@HertzDevil HertzDevil deleted the bug/windows-deallocationstack branch September 26, 2024 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows topic:stdlib:concurrency
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants