Skip to content

Conversation

@kinke
Copy link
Member

@kinke kinke commented May 23, 2021

@kinke
Copy link
Member Author

kinke commented May 24, 2021

@rainers: Do you happen to have insights regarding CRT dtors and DLLs? The first hurdle I'm seeing here wrt. loading and unloading DLLs at runtime is that the CRT ctors are invoked just fine, but the dtors aren't after FreeLibrary (the library is correctly unloaded though, doesn't show up in the VS modules list anymore). After a quick glance at the CRT's DllMain source code, the dtors appear to be skipped only when the process is about to terminate. I thought I'd ask before experimenting with a custom DllMain...

@kinke
Copy link
Member Author

kinke commented May 24, 2021

Oh, I've obviously looked at the wrong DllMain source - the correct one is in C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\crt\src\vcruntime\dll_dllmain.cpp, not in C:\Program Files (x86)\Windows Kits\10\Source\10.0.17763.0\ucrt\dll\appcrt_dllmain.cpp. According to the schematics in that correct file, it looks like skipping the dtors is indeed according to someone's plan. ;)

Registering the 'dtor' manually via atexit() seems to work just fine for now...

@rainers
Copy link
Contributor

rainers commented May 25, 2021

Oh, I've obviously looked at the wrong DllMain source

There is also c:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29910\crt\src\vcruntime\vcruntime_dllmain.cpp which is used if you link the VC runtime statically AFAICT.

Registering the 'dtor' manually via atexit() seems to work just fine for now...

IIRC there is a DLL local list of atexit funtions to call on unload. Didn't we implement that in the mingw-based DLLs?

@kinke
Copy link
Member Author

kinke commented May 25, 2021

IIRC there is a DLL local list of atexit funtions to call on unload.

Exactly, and that's why it works just fine.

Didn't we implement that in the mingw-based DLLs?

Yes - https://github.com/ldc-developers/mingw-w64-libs/blob/33aa1e813061cde47d2a66635eccb8b09c427f71/msvcrt_stub.c#L59. I haven't tested the new DLLs with those libs at all though.


Druntime's shared/dllgc integration test still sporadically fails - the dllgc.dll might be unloaded before the extra thread spawned in https://github.com/ldc-developers/druntime/blob/570194c3d6a4e4206faa8bea4511876acf368386/test/shared/src/dllgc.d#L39 is finished. I've looked a bit at the lowLevelThread Windows specifics yesterday and noticed that druntime takes care to increase the DLL ref count to prevent that (apparently superfluous for UCRT), but assumes that the thread code and callback are in the same binary as druntime. I don't think this problem can be solved with a truly shared druntime DLL (unloading a DLL manually while threads are still executing code from that DLL), so I tend to just disable that test.

I've also tried to radically simplify the SimpleDllMain druntime mixin (used by 'dllgc.dll'), only initializing the runtime at process attach and terminating it at process detach for version(Shared), with the same results. I guess it was you who implemented it; do you know/remember whether any extra stuff in there is still required for a separate druntime DLL?

@rainers
Copy link
Contributor

rainers commented May 26, 2021

I don't think this problem can be solved with a truly shared druntime DLL (unloading a DLL manually while threads are still executing code from that DLL), so I tend to just disable that test.

A shared D runtime DLL will contain core.Thread, but not the actual thread code, so that mechanism has to be adapted (or ignored) anyway.

only initializing the runtime at process attach and terminating it at process detach for version(Shared)

While threads should only be attached to by the shared runtime, thread local data in the DLL still needs to be initialized. The flags passed to dll_process/thread_attach/detach are meant to be used in this case.

@kinke
Copy link
Member Author

kinke commented May 26, 2021

While threads should only be attached to by the shared runtime, thread local data in the DLL still needs to be initialized. The flags passed to dll_process/thread_attach/detach are meant to be used in this case.

AFAICT, this is all working just fine without these extra hooks - if the thread is started via core.thread and DLLs being loaded either implicitly or via Runtime.loadLibrary(). To me, it looked like a way to initialize TLS data of threads spawned in another druntime - e.g., initialize DLL TLS data for threads spawned in the .exe, with its own druntime (and threads list).

@rainers
Copy link
Contributor

rainers commented May 26, 2021

AFAICT, this is all working just fine without these extra hooks

Ok, that might work because you now register the DLL in the D runtime DLL in ldc-developers/druntime#197

@kinke
Copy link
Member Author

kinke commented May 26, 2021

Ah yeah sorry, should have emphasized that the 3 extra Windows tests are now tested with shared druntime.dll, not static druntime anymore. - I'll just use the static one for the dllgc test, problem solved. :)

@kinke kinke merged commit cb97aa0 into ldc-developers:master May 27, 2021
@kinke kinke deleted the test_shared branch May 27, 2021 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants