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

Fix detection of thread-local storage capability for MSVC #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mapret
Copy link

@mapret mapret commented Aug 25, 2023

This PR fixes an incorrect check for detecting whether MSVC supports thread-local storage or not.

The build system uses the cmake function try_compile to compile a test file which contains extern __thread int x, but this code always fails with MSVC since the correct thread-local attribute would be __declspec(thread) (Source from Microsoft). The __thread keyword is redefined for MSVC in cmake for the library, but this does not affect this call to try_compile.
This then leads to the thread-local attribute being omitted for MSVC builds, which in turn leads to crashes when calling into the library from multiple threads at the same time.

I tested this change with the latest MSVC compiler by writing a small test program with calls METIS_PartGraphRecursive inside a std::for_each(std::execution::par, ...) loop. Without this change the code crashes even for just two parallel executions, with the change it runs successfully for a larger number of parallel executions.

Since the file conf/check_thread_storage.c is only used by the build system when the MSVC compiler is being used, it should be fine to change it like this.
A PR with the same fix was also supplied to the METIS repository (METIS/pull/74)

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.

1 participant