Skip to content

Conversation

HertzDevil
Copy link
Contributor

LibC.SetThreadStackGuarantee only works on the current thread. This PR extends it to other threads created explicitly or via -Dpreview_mt, so that stack overflows on them will print a stack trace properly:

def foo
  x = uninitialized UInt8[512]
  foo
end

Thread.new { foo }.join
Stack overflow (e.g., infinite or very deep recursion)
[0x7ff79edc1fb7] foo at C:\Users\nicet\crystal\crystal\usr\test.cr:28
[0x7ff79edc1fbc] foo at C:\Users\nicet\crystal\crystal\usr\test.cr:28 (14843 times)
[0x7ff79edc1fa9] -> at C:\Users\nicet\crystal\crystal\usr\test.cr:31
[0x7ff79ee365c3] start at C:\Users\nicet\crystal\crystal\src\crystal\system\thread.cr:116
[0x7ff79ee36957] thread_proc at C:\Users\nicet\crystal\crystal\src\crystal\system\win32\thread.cr:30
[0x7ff79edc1fc9] -> at C:\Users\nicet\crystal\crystal\src\crystal\system\win32\thread.cr:15
[0x7ff79ee75766] GC_wait_marker +454 in C:\Users\nicet\crystal\crystal\test.exe
[0x7ff79ee75a7a] GC_call_with_stack_base +26 in C:\Users\nicet\crystal\crystal\test.exe
[0x7ff79eeacfd6] thread_start<unsigned int (__cdecl*)(void *),1> at minkernel\crts\ucrt\src\appcrt\startup\thread.cpp:97
[0x7fff3525257d] BaseThreadInitThunk +29 in C:\WINDOWS\System32\KERNEL32.DLL
[0x7fff3640aa58] RtlUserThreadStart +40 in C:\WINDOWS\SYSTEM32\ntdll.dll

@HertzDevil HertzDevil added 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:multithreading labels Jan 8, 2024
@beta-ziliani beta-ziliani added this to the 1.12.0 milestone Jan 8, 2024
@Fryguy
Copy link
Contributor

Fryguy commented Jan 9, 2024

For posterity, can you update the OP to show what the stack trace looked like before your change?

@HertzDevil
Copy link
Contributor Author

Stack overflow (e.g., infinite or very deep recursion)
Stack overflow (e.g., infinite or very deep recursion)

@straight-shoota straight-shoota changed the title Windows: Reserve stack space on non-main threads for crash recovery Reserve stack space on non-main threads for crash recovery on Windows Jan 12, 2024
@straight-shoota straight-shoota merged commit f5844b0 into crystal-lang:master Jan 12, 2024
@Fryguy
Copy link
Contributor

Fryguy commented Jan 12, 2024

Ah thanks...I thought it was more complicated than that 😆

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:multithreading topic:stdlib:concurrency

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants