Skip to content
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

Makefile: Add a dependency on $(LDLIBRARY) for the sharedmods target #163

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

mstorsjo
Copy link

@mstorsjo mstorsjo commented Dec 4, 2023

When building shared modules (*.pyd) for MinGW, they link against the import library for the main python (e.g. -lpython3.11). When linking shared modules on Linux, this doesn't happen, but those shared libraries are linked with undefined references which then are fulfilled by the host process when they are loaded.

Add a dependency to make sure that $(LDLIBRARY), e.g. libpython$(LDVERSION).dll.a, exists before starting to build the shared modules.

Previously, if libpython$(LDVERSION).dll.a didn't exist when the shared modules were linked, some of them failed to link, but this wasn't reported as an error in the build, and a later invocation of setup.py would retry building them, which then would succeed. E.g. there was a seemingly benign race condition in the build.

However, in some rare cases, the race condition no longer was benign. The build also produces a corresponding static library, $(LIBRARY), e.g. libpython$(VERSION)$(ABIFLAGS).a - which in the end would be named similarly, libpython3.11.a, when $(LDLIBRARY) would be libpython3.11.dll.a.

If the static library exists but the import library doesn't, then most modules would still fail to link, but a few ones wouldn't (select, _multiprocessing, _overlapped and _socket). The ones that did succeed linking with the static library would crash at runtime though.

By making sure that the right, intended library to fulfill the linker parameter -lpython3.11 exists before building the shared modules, we avoid the whole race condition that in some rare cases could produce a Python build that crashes at runtime.

For Linux targets, this extra dependency is unnecessary, but should not cause other problems ($(LDLIBRARY) is set to the same as $(LIBRARY) if not building any shared library).

This fixes MSYS2 cpython-mingw issue #162.

When building shared modules (*.pyd) for MinGW, they link against
the import library for the main python (e.g. -lpython3.11). When
linking shared modules on Linux, this doesn't happen, but those
shared libraries are linked with undefined references which
then are fulfilled by the host process when they are loaded.

Add a dependency to make sure that $(LDLIBRARY), e.g.
libpython$(LDVERSION).dll.a, exists before starting to build
the shared modules.

Previously, if libpython$(LDVERSION).dll.a didn't exist when
the shared modules were linked, some of them failed to link,
but this wasn't reported as an error in the build, and a
later invocation of setup.py would retry building them, which
then would succeed. E.g. there was a seemingly benign race
condition in the build.

However, in some rare cases, the race condition no longer
was benign. The build also produces a corresponding static library,
$(LIBRARY), e.g. libpython$(VERSION)$(ABIFLAGS).a - which in
the end would be named similarly, libpython3.11.a, when
$(LDLIBRARY) would be libpython3.11.dll.a.

If the static library exists but the import library doesn't, then
most modules would still fail to link, but a few ones wouldn't
(select, _multiprocessing, _overlapped and _socket). The ones
that did succeed linking with the static library would crash at
runtime though.

By making sure that the right, intended library to fulfill the
linker parameter "-lpython3.11" exists before building the shared
modules, we avoid the whole race condition that in some rare
cases could produce a Python build that crashes at runtime.

For Linux targets, this extra dependency is unnecessary, but should
not cause other problems ($(LDLIBRARY) is set to the same as
$(LIBRARY) if not building any shared library).

This fixes MSYS2 cpython-mingw issue msys2-contrib#162.
@lazka lazka merged commit 03d9f3c into msys2-contrib:mingw-v3.11.6 Dec 4, 2023
13 checks passed
@mstorsjo mstorsjo deleted the shared-mods-deps branch December 5, 2023 11:36
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