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

Loading debug symbols is not thread-safe on Windows #14952

Open
HertzDevil opened this issue Aug 30, 2024 · 3 comments
Open

Loading debug symbols is not thread-safe on Windows #14952

HertzDevil opened this issue Aug 30, 2024 · 3 comments
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:runtime

Comments

@HertzDevil
Copy link
Contributor

On Windows, when debug symbols are loaded concurrently from multiple threads, such as when raising an exception or calling caller, most of those threads will fail. A snippet like:

4.times do
  Thread.new do
    puts "#{caller.inspect}\n"
  end
end
puts "#{caller.inspect}\n"
sleep 0.1

could produce results like:

["???", "???", "???"]
["???", "???", "???"]
["???", "???", "???"]
["???", "???", "???"]
["usr\\test.cr:27 in '__crystal_main'", "src\\crystal\\main.cr:118 in 'main_user_code'", "src\\crystal\\main.cr:104 in 'main'", "src\\crystal\\main.cr:130 in 'main'", "src\\crystal\\system\\win32\\wmain.cr:37 in 'wmain'", "D:\\a\\_work\\1\\s\\src\\vctools\\crt\\vcstartup\\src\\startup\\exe_common.inl:288 in '__scrt_common_main_seh'", "C:\\WINDOWS\\System32\\KERNEL32.DLL +75133 in 'BaseThreadInitThunk'", "C:\\WINDOWS\\SYSTEM32\\ntdll.dll +372520 in 'RtlUserThreadStart'"]
["???", "???", "???"]
Unable to load debug information["???", "???", "???"]
Invalid memory access (C0000005) at address : SymInitializeW: The parameter is incorrect. (RuntimeError)
0x["???", "???", "???"]
1f565411000
  from ???
  from ???[0x7ff73a44789c] 
  from ???
  from ???
???
  from ???
  from [0x1fffffff0000000d] ???
[0x7ff73a446163] ???
???
[["???"]
0xd00000000] ???
[0x52933d3fd4da2e04] ???
[0xe9fe19dd54ddbdb6] ???
[0x7ff73a4475e6] ???
[0x1f5653d0d00] ???
[0x6258e96d0000000d] ???
[0x1a36ad96b7e88ea] ???
[0x1f5653dcd80] ???
[0xab200000001] ???
[0x100000001] ???
[0x1] ???
[0x1f5653dcd80] ???
[0x97980500979805] ???
[0x1f5653d0d00] ???

This is also reproducible with fibers under -Dpreview_mt:

4.times do
  spawn do
    puts "#{caller.inspect}\n"
  end
end
puts "#{caller.inspect}\n"
sleep 0.1

All related Win32 functions are single-threaded; replacing Exception::CallStack.@@sym_loaded with an Atomic::Flag is not enough, because those alternate threads still end up calling LibC.StackWalk64 to unwind their stacks before LibC.SymInitializeW returns. Therefore, all threads must await the completion of Exception::CallStack.load_debug_info_impl on whatever thread it is called.

To achieve this, #14905 or something like InitOnceExecuteOnce should be used instead.

@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:stdlib:runtime topic:multithreading labels Aug 30, 2024
@ysbaddaden
Copy link
Contributor

What about a mutex in the meantime?

@HertzDevil
Copy link
Contributor Author

HertzDevil commented Aug 31, 2024

Loading symbols is also not thread-safe even after they have been initialized:

caller
Array.new(4) do
  Thread.new do
    100.times { caller }
  end
end.each(&.join)
# `-Dpreview_mt`
require "wait_group"

caller
WaitGroup.wait do |wg|
  4.times do
    wg.spawn do
      100.times { caller }
    end
  end
end

Eventually Exception::CallStack.decode_line_number will return bogus strings that include null characters at the end. So much of Exception::CallStack#decode_backtrace probably needs to be in a mutex as well.

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:runtime
Projects
Status: Todo
Development

No branches or pull requests

3 participants
@ysbaddaden @HertzDevil and others