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

file.c: fix uninitialized mutex in MPI_File handle #12416

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

jsquyres
Copy link
Member

@jsquyres jsquyres commented Mar 19, 2024

We were initializing the mutex in file_open, but that didn't handle MPI_FILE_NULL. So we move it to the constructor, and therefore it's always initialized for all file handles -- even MPI_FILE_NULL.

Thanks to mpi4py test suite for finding this one. 👍

@jsquyres
Copy link
Member Author

@edgargabriel Found another issue; I fixed and re-pushed. @bosilca saw the fix in Slack and approved it there (so even though my push is after his github approval, he still approved 👍).

Copy link
Member

@bosilca bosilca left a comment

Choose a reason for hiding this comment

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

we still need to destruct this lock.

@jsquyres jsquyres marked this pull request as draft March 19, 2024 20:12
We were initializing the mutex in file_open, but that didn't handle
MPI_FILE_NULL.  So we move it to the constructor, and therefore it's
always initialized for all file handles -- even MPI_FILE_NULL.

Signed-off-by: George Bosilca <bosilca@icl.utk.edu>
Signed-off-by: Jeff Squyres <jeff@squyres.com>
@jsquyres jsquyres marked this pull request as ready for review March 20, 2024 00:06
@jsquyres
Copy link
Member Author

@bosilca @edgargabriel Please re-review.

@jsquyres
Copy link
Member Author

The continuous-integration/jenkins/branch failed CI is because I accidentally pushed this branch to the main github repo (not my fork), and then I deleted the branch. Hence, Jenkins tried to build it, but failed. This false CI failure can be ignored.

@jsquyres jsquyres merged commit e9a0e65 into open-mpi:main Mar 20, 2024
11 of 12 checks passed
@jsquyres jsquyres deleted the pr/mpi-file-mutex-fix branch March 20, 2024 01:11
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.

2 participants