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

Report error and abort in CollectiveMutex during stack unwinding #14356

Merged
merged 2 commits into from Nov 1, 2022

Conversation

gassmoeller
Copy link
Member

When we throw an exception inside a CollectiveMutex::ScopedLock the destructor of ScopedLock tries to unlock the CollectiveMutex. This can lead to hard to debug MPI deadlocks, because other MPI ranks are likely trying to communicate with the ranks that throw the exception, while the ranks that throw the exceptions wait in the MPI barrier to unlock the mutex. We should not try to communicate while an exception is uncaught, because we do not know if all processes threw or just a subset.

The current behavior made it very hard to debug geodynamics/aspect#4984.

@gassmoeller gassmoeller changed the title Remove communication in CollectiveMutex during stack unwinding [WIP] Remove communication in CollectiveMutex during stack unwinding Oct 18, 2022
@gassmoeller
Copy link
Member Author

Actually, let me mark this WIP for now. Even with this change we run into a deadlock. I confirmed that we do not get into it if I call MPI_Abort in case an exception is uncaught, but I did not include that in this PR (because I want the exception to propagate up).

I first want to investigate where we get stuck, but afterwards: Would it be fair to say that if an exception is raised within a CollectiveMutex, we cannot recover anyway, because other ranks are likely waiting for communication? Should we make sure we do not run into a deadlock in this case by calling MPI_Abort (possibly after printing an error and the exception)? We do this already in the HDF5 output here. Or would this be the responsibility of the exception handler that catches the exception further up the call stack?

@tjhei
Copy link
Member

tjhei commented Oct 18, 2022

Yes, I would do the same as what we do for HDF5 and for TimerOutput. Communication in a destructor is not going to be a recoverable situation, I think.

@bangerth
Copy link
Member

I agree.

source/base/mpi.cc Outdated Show resolved Hide resolved
@gassmoeller gassmoeller changed the title [WIP] Remove communication in CollectiveMutex during stack unwinding Report error and abort in CollectiveMutex during stack unwinding Oct 20, 2022
@gassmoeller
Copy link
Member Author

Ok, I think I made the requested changes. Now we should abort whenever a CollectiveMutex is destroyed or unlocked (from a ScopedLock) when an exception is thrown. Let me know if you want me to change the error message.

Copy link
Member

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

@drwells
Copy link
Member

drwells commented Oct 20, 2022

/rebuild

@drwells drwells merged commit d89583e into dealii:master Nov 1, 2022
@gassmoeller gassmoeller deleted the avoid_deadlock_in_mutex branch May 11, 2023 14:13
@gassmoeller gassmoeller restored the avoid_deadlock_in_mutex branch April 19, 2024 14:32
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.

None yet

5 participants