wxWidgets / wxWidgets

Cross-Platform C++ GUI Library
https://www.wxwidgets.org/
6.18k stars 1.77k forks source link

wxWidgets crashs on Windows when load as DLL #24730

Closed starwing closed 3 months ago

starwing commented 3 months ago

Description

Bug description:

  1. load wx.dll with LoadLibraryA: image

  2. unload dll with FreeLibrary

  3. call ExitProcess

Expected vs observed behaviour:

process crashs in nt.dll, RtlpFlsDataCleanup

Stack trace:

image

Patch or snippet allowing to reproduce the problem:

minimal example:

example.zip

To Reproduce:

unzip the example.zip, and runs lua54.exe test.lua

Platform and version information

vadz commented 3 months ago

It would be really great to have a minimal example, e.g. not involving Lua. Please write a tiny program calling LoadLibrary() and FreeLibrary() from C if you can.

PBfordev commented 3 months ago

AFAICT, this is not a wxWidgets DLL.

Firstly, the name: wx.dll. This does not match the wxWidgets DLL naming pattern.

Secondly, looking at the DLL details in Explorer shows it is a wxLua library: wx-dll-lua

Lastly, program Dependencies shows it does not export wxWidgets classes, only wxLua (e.g., wxLuaModuleApp) and it depends on lua54.dll.

All this means that you should report the issue to wxLua maintainers, not wxWidgets ones.

BTW, attempting to open this DLL in Dependency Walker (DW) makes DW freeze. I don't think I ever saw that with any other DLL (but I don't open that many in DW).

vadz commented 3 months ago

Thanks, I didn't even realize wx.dll was the actuall DLL name and not a shortcut for "wxWidgets DLL".

We absolutely need a way to reproduce the problem using wx sources only, we can't do anything about any problems in DLLs that are not built by us.

starwing commented 3 months ago

Okay, I will try to build a pure wxWidgets DLL and a pure C program for it. Sorry for the noise from Lua binding (I'm noticing this strange behavior when I trying to make a distro of some Lua bindings).

PBfordev commented 3 months ago

FWIW, running this code built with MSVS 2022 as 64-bit console application on Windows 10

#include <windows.h>

int main()
{
    HMODULE wxDLL = LoadLibraryW(L"wx.dll");

    if ( !wxDLL )
    {
        LPVOID msgBuf;
        DWORD  bufLen = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM ,
                                       nullptr, GetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPWSTR)&msgBuf, 0, nullptr);
        if ( bufLen )
        {
            MessageBoxW(nullptr, (LPCWSTR)msgBuf, L"ERROR", MB_OK | MB_ICONERROR);
            LocalFree(msgBuf);
        }
        else
        {
            MessageBoxW(nullptr, L"LoadLibrary(wx.dll) failed.", L"ERROR", MB_OK | MB_ICONERROR);
        }
    }
    else
    {
        FreeLibrary(wxDLL);
    }
}

produces only the message box with an error message The specified module could not be found (probably lua54.dll). This IMO further confirms that the issue is in the lua54.DLL (in its DllMain()?) and nothing do with wxWidgets itself.

starwing commented 3 months ago

@PBfordev I have tried this test program and it works well (I produced a lua54.dll for it). I think it's expected behavior. the wx.dll is from a Lua binding project for wxWidgets (wxLua).

The wxLua project static links the vs static libraries of wxWidgets and its binding code into one single dll, thus wx.dll.

When it loaded from Lua, it's luaopen_wx will be called, and some code runs. After that (I think) some fibers are created, but not freed when unload the DLL, so the ExitProcess tries to free them and accessed the freed DLL address space and cause the crash.

I will look into the luaopen_wx code and finding out what does it runs. And (maybe) report this bug to wxLua project. Before that I will try to build a DLL from wxWidgets static libraires, and run the codes that luaopen_wx runs, and see whether it crash or not.

starwing commented 3 months ago

@vadz @PBfordev this is a real minimal example: wx-test.zip

Runs host.exe to see the crash in ntdll.dll to build it, change the wxdir in build.bat and run. the wxWidgets used built as:

mkdir wx-build && cd wx-build
cmake -DwxBUILD_USE_STATIC_RUNTIME=ON -DCMAKE_INSTALL_PREFIX=%CD%/../wx-dist ../wxWidgets-3.2.5
cmake --build . --config Release
cmake --install .
vadz commented 3 months ago

Is static runtime required to reproduce the problem?

P.S. Also, the ZIP still includes wx.dll, what is this and where does it come from? The example can't be really called minimal when it includes external DLLs...

starwing commented 3 months ago

Is static runtime required to reproduce the problem?

P.S. Also, the ZIP still includes wx.dll, what is this and where does it come from? The example can't be really called minimal when it includes external DLLs...

I can try not static linking. I'm just used to doing it this way, so that my dll can run on more systems.

The wx.dll is built from dll.cpp, I just include all binaries for others could run it directly.

starwing commented 3 months ago

Is static runtime required to reproduce the problem?

I have tried, dynamic linked MSVCP140D.dll still crashed in ntdll.dll.

image

b-fission commented 3 months ago

You might want to change wxUSE_CMDLINE_PARSER to 0 through your cmake config, or in include/wx/msw/setup.h and then rebuild wxWidgets.

It might be related to argc/argv being passed as 0/null from entry(). Alternatively, you can omit wxApp::OnInit() from your app code since the command-line parser happens there.

Either way should fix the crash on exit.

starwing commented 3 months ago

@b-fission Thanks for the reply! it works when build wxWidgets without wxUSE_CMDLINE_PARSER support!

but wxLua uses the wxUSE_CMDLINE_PARSER, disable it makes wxLua build fail :-(

Is there any workaround to pass valid argc/argv in entry() makes me patch the wxLua code to work?

EDIT: And It seems not the root case. I have patched wxLua code and remove wxUSE_CMDLINE_PARSER need, and makes a new wx.dll to run auidemo.wx.lua sample, it works well, but still crashs on ntdll.dll when exit: image

b-fission commented 3 months ago

Have you tried editing this line from luamodule.cpp?

Change it from return wxApp::OnInit(); to return true;

starwing commented 3 months ago

Have you tried editing this line from luamodule.cpp?

Change it from return wxApp::OnInit(); to return true;

change it to return true still crashes on exit. I feel like if we call any functions from wxWidgets in DLL, then the ntdll.dll will crash on exit, and it's always crashes on RtlpFlsDataCleanup, I think the root cause should find out to solve this protblem.

vadz commented 3 months ago

Thanks, I think I understand what's going on here, but I don't see a good solution yet, unfortunately.

The problem is that, since e19984e2dd8bfc1a9f96477feb61d6029a7694f8, we use FlsAlloc() which calls our function to free memory when the process terminates, but in this case this happens after wx code is not present in memory any more because the DLL got unloaded, and so calling the callback crashes. We need to somehow ensure that we call FlsFree() on DLL unload, but I don't know how can we do it when static wx lib is linked as part of another DLL.

Right now my only idea is to revert the commit above (after reverting cf66599eba39810d40da01a03d0ec87a4b03a3d2 too).

cc @nietsu in case you have any ideas about how to fix this

starwing commented 3 months ago

@vadz Thanks for the root cause! I can run any code when the DLL unloaded (from DllMain), So it's good for me to add a function which let user call when the DLL unloaded.

vadz commented 3 months ago

While it's true that this would be relatively simple to fix once you know you have to call some function on DLL unload, I'm afraid it could still result in a lot of aggravation when the application starts crashing after updating wx and before you find the right function to call, so I'm reverting this entirely for 3.2.6, see #24747. If you merge the changes from there, this crash shouldn't happen any longer.

starwing commented 3 months ago

@vadz Thanks for the fix! it works like a charm, the DLL doesn't crashes on exit now.

But I have some extra issues about the wxWidgets binding :-(

I use the MemoryModule to load wx.dll for inject the depends of lua54.dll into lua executable itself, but It crashs on wxEntryStart, it seems crashes on the DoCommonPostInit, RegisterModule and the register of wxDDEModule. But it seems a bug of MemoryModule itself. Loading wx.dll with just LoadLibraryA works well, I will look into MemoryModule and wxDDEModule to find out why it crashes.

Anyway, Thanks the great fix!

starwing commented 3 months ago

I have found out the reason of the wxWidgets crashes on MemoryModule. (FYI, this issue). MemoryModule doesn't support TLS initialize when load DLLs, makes wxCriticalSection initialize failed. I will try to patch it to support it, but that's not caused by wxWidgets. So thanks all! This issues now solved complete!