-
Notifications
You must be signed in to change notification settings - Fork 168
Fix Win32 multithread dispatching bugs. #265
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this, it's very much appreciated.
FYI: since the patch forced using dispatch table on Win32 the compiler will complain about unused
(See CI log in https://github.com/anholt/libepoxy/runs/4226313433?check_suite_focus=true). |
@ebassi Could you support him further with his question, so this can be merged eventually? |
This pull request resolved some issues I was seeing around the dispatch returning null for function pointers when accessed from different threads |
- Resolve multithreading issues - Enable building as a static library Ref: anholt/libepoxy#265
- Resolve multithreading issues - Enable building as a static library Unfortunately, using it as a static library causes linking errors, so we're keeping the line that forces it to be a dynamic library on Windows. Ref: anholt/libepoxy#265
- Resolve multithreading issues - Enable building as a static library Unfortunately, using it as a static library causes linking errors, so we're keeping the line that forces it to be a dynamic library on Windows. Ref: anholt/libepoxy#265
Is there any progress on this matter ? I am too seeing crashes with Windows builds (no issue on Linux) when trying to create and switch to shared GL contexts, with a Second Life third party viewer I am trying to migrate to libepoxy... EDIT: I applied the "threaded.patch" from dpogue/Plasma@16149d7 and it indeed seems to solve the crashes I was seeing with libepoxy under Windows (more testing will be needed, but so far, so good). |
One way to avoid compiler warnings would be to mark everything with |
Since I'm not a Windows expert or developer, I'd like to get a second pair of eyes; switching to thread-local storage using compiler annotations instead of run time API is a bit iffy; I don't want to regress the shared library use case on Windows just to fix the static build case. I honestly think that static builds of C library in 2022 are a mistake more often than not, anyway. I care about static builds insofar as they don't require weird contortions. |
As a follow up to my previous comment, I must point out that while the use of multiple shared GL contexts do not crash any more libepoxy with that patch, I am faced with a weird issue, where the viewer process never exits on WinMain() return ! |
Not yet completely tested but there is a crash on Windows and it should be solved by this patch. Based on: anholt/libepoxy#200 anholt/libepoxy#265
Glew has the problem that it has to be selected at build time if GLX or EGL is supported by the library, and this in not encoded in the library name, nor ABI, nor anything. Then it's easy to get into the situation that a binary is built but cannot run because glew supports an API different from the one used by wxWidgets, or the binary fails to link in the end after all objects are compiled. epoxy can support both with the same library avoiding this problem. epoxy is not initialized explicitly, replaced initialization with version check where one was not done already. It seems to be available as vcpkg https://vcpkg.link/ports/libepoxy There are problems related to GL context switching on Windows which does not seem to be used in kicad https://github.com/anholt/libepoxy#known-issues-when-running-on-windows There is also a problem related to multithreaded rendering on Windows anholt/libepoxy#265 It's harder to tell if threading is used for rendering but it does not look like kicad is doing anything complex enough to warrant using multiple rendering threads. Fixes https://gitlab.com/kicad/code/kicad/-/issues/20630 Fixes https://gitlab.com/kicad/code/kicad/-/issues/12543
Using
__thread
(and__declspec(thread)
on MSVC) to replace dirty DllMain hack and thus allowing static build (-Ddefault_library=static
). (Possibly solving #200 issue.)Also fixed race condition in dispatch table logic. Now on Win32 it will always use dispatch table. (#199 minimal reproducing example passed.)