terminal icon indicating copy to clipboard operation
terminal copied to clipboard

CreateThread should be replaced with _beginthreadex

Open lhecker opened this issue 3 years ago • 2 comments

Funny thing I just discovered:

A thread in an executable that calls the C run-time library (CRT) should use the _beginthreadex and _endthreadex functions for thread management rather than CreateThread and ExitThread; this requires the use of the multithreaded version of the CRT. If a thread created using CreateThread calls the CRT, the CRT may terminate the process in low-memory conditions.

For correctness' sake we should use std::thread (or _beginthreadex) throughout the project instead of CreateThread if possible.

lhecker avatar Apr 08 '22 23:04 lhecker

Thanks for filing this! It has been kicking around in the back of my head for a while.

DHowett avatar Apr 08 '22 23:04 DHowett

FYI because of the PR just now I checked what the differences are:

  • It calls RoInitialize(RO_INIT_MULTITHREADED) depending on the return value of https://learn.microsoft.com/en-us/windows/win32/api/appmodel/nf-appmodel-apppolicygetthreadinitializationtype Why, Microsoft, why? What does COM got to do with C? In any case, in practice this means we get an apartment on every thread within Windows Terminal but not within conhost. I'm not sure what the cost of a RoInitialize call is.
  • Calls GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS) on start and FreeLibraryAndExitThread on exit for us to ensure DLLs remain loaded. (Neat?)
  • Wraps the call in a __try / __except. (Neat?)

It seems unlike what SO says, at some point the code was changed so that the per-thread-data was deallocated when the CRT DLL unloads (search for the __acrt_thread_detach in the CRT source folder). What I wonder now is how it does that if it's statically linked... But I guess that doesn't affect us.

lhecker avatar May 12 '24 19:05 lhecker