oneTBB icon indicating copy to clipboard operation
oneTBB copied to clipboard

Unsafe DllMain usage

Open jermarti opened this issue 5 years ago • 11 comments

I left a summary of this on the tbb forums last week, but it hasn't got any traction so i thought i'd cross post here too.

The issue is the current use of callbacks from the scheduler framework (at least when using flow graphs) is unsafe for many operations, and even runs against safe DllMain practices listed on this Intel blog post.

Here's an example stack that eventually calls into a continue node callback:

tbb_debug.dll!tbb::internal::custom_schedulertbb::internal::IntelSchedulerTraits::local_wait_for_all(tbb::task & parent, tbb::task * child) Line 517 C++ tbb_debug.dll!tbb::internal::generic_scheduler::cleanup_master(bool blocking_terminate) Line 1317 C++ tbb_debug.dll!tbb::internal::governor::auto_terminate(void * arg) Line 222 C++ tbb_debug.dll!tbb::internal::governor::terminate_auto_initialized_scheduler() Line 139 C++ tbb_debug.dll!DllMain(void * __formal, unsigned long reason, void * lpvReserved) Line 264 C++

In short, it appears that an invocation of DllMain can call back into user callbacks on graph nodes, which is unsafe because while a DllMain is open a loader lock is established in windows. There's an entire family of operations on windows that can deadlock if done while inside a loader lock, and that blog post has a pretty good list (republished here):

  • Call LoadLibrary or LoadLibraryEx (either directly or indirectly). This can cause a deadlock or a crash.

  • Call GetStringTypeA, GetStringTypeEx, or GetStringTypeW (either directly or indirectly). This can cause a deadlock or a crash.

  • Synchronize with other threads. This can cause a deadlock.

  • Acquire a synchronization object that is owned by code that is waiting to acquire the loader lock. This can cause a deadlock.

  • Initialize COM threads by using CoInitializeEx Under certain conditions, this function can call LoadLibraryEx.

  • Call the registry functions. These functions are implemented in Advapi32.dll. If Advapi32.dll is not initialized before your DLL, the DLL can access uninitialized memory and cause the process to crash.

  • Call CreateProcess. Creating a process can load another DLL.

  • Call ExitThread. Exiting a thread during DLL detach can cause the loader lock to be acquired again, causing a deadlock or a crash.

  • Call CreateThread. Creating a thread can work if you do not synchronize with other threads, but it is risky.

  • Create a named pipe or other named object (Windows 2000 only). In Windows 2000, named objects are provided by the Terminal Services DLL. If this DLL is not initialized, calls to the DLL can cause the process to crash.

  • Use the memory management function from the dynamic C Run-Time (CRT). If the CRT DLL is not initialized, calls to these functions can cause the process to crash.

  • Call functions in User32.dll or Gdi32.dll. Some functions load another DLL, which may not be initialized.

  • Use managed code.

I'm currently running into this in a project that needs to call back into managed code on a per node basis. I've confirmed this behavior persists in the TBB 2019 Update 8 release.

Optimally the solution here would be for DllMain to become a stub that tracks lifetime, and doesn't stay open due to the unsafe rules around window's loader lock. I realize that may not be possible and other solutions could be done too, and i'm very open to work arounds for this issue.

At the very least if there's some reason why TBB needs to both keep DllMain open AND call back into user code from it, that should be documented as an important consideration for developers evaluating the scheduler framework.

jermarti avatar Aug 13 '19 19:08 jermarti