Skip to content

bugfix: Setting OMPI_MPI_THREAD_LEVEL to a value different than requested crashes #13211

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

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

Conversation

abouteiller
Copy link
Member

Setting OMPI_MPI_THREAD_LEVEL to a value different than requested in MPI_Init_thread would invoke the error handler, even though it is an useful override in some threaded library use cases.

@abouteiller abouteiller removed the bug label Apr 25, 2025
@abouteiller abouteiller force-pushed the bugfix/env-thread-level-ignored branch 4 times, most recently from b71461f to b481893 Compare April 25, 2025 16:29
@abouteiller abouteiller requested review from jsquyres and bosilca May 5, 2025 15:33
@@ -46,7 +47,8 @@ PROTOTYPE INT init(INT_OUT argc, ARGV argv)

if (NULL != (env = getenv("OMPI_MPI_THREAD_LEVEL"))) {
required = atoi(env);
if (required < MPI_THREAD_SINGLE || required > MPI_THREAD_MULTIPLE) {
if (required != MPI_THREAD_SINGLE && required != MPI_THREAD_FUNNELED &&
required != MPI_THREAD_SERIALIZED && required != MPI_THREAD_MULTIPLE) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: what's the need for this? Doesn't MPI specify the relationship between all the MPI_THREAD_* values such that < SINGLE and > MULTIPLE is guaranteed to be invalid?

Copy link
Member Author

@abouteiller abouteiller May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the new MPI ABI, values are still ordered but not consecutive, so users could use one of the invalid intermediate values.

https://github.com/mpi-forum/mpi-abi-stubs/blob/e89a80017a3fe9a05d903ced2564c6342d678165/mpi.h#L334

@abouteiller abouteiller force-pushed the bugfix/env-thread-level-ignored branch from b481893 to 1a0d2f7 Compare May 12, 2025 17:42
Copy link

Hello! The Git Commit Checker CI bot found a few problems with this PR:

1a0d2f7: review comments

  • check_signed_off: does not contain a valid Signed-off-by line

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

`requested` in `MPI_Init_thread` would invoke the error handler, even
though it is an useful override in some threaded library use cases.

Signed-off-by: Aurelien Bouteiller <abouteil@amd.com>
@abouteiller abouteiller force-pushed the bugfix/env-thread-level-ignored branch from 1a0d2f7 to 8ad7579 Compare May 12, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OMPI_MPI_THREAD_LEVEL cannot override 'required' in MPI_Init_thread
3 participants