Skip to content

Update DeallocationStack for Windows context switch#15032

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:bug/windows-deallocationstack
Sep 26, 2024
Merged

Update DeallocationStack for Windows context switch#15032
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:bug/windows-deallocationstack

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 Windows support based on the MSVC toolchain / Win32 API 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
@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 Windows support based on the MSVC toolchain / Win32 API topic:stdlib:concurrency

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants