Skip to content
This repository was archived by the owner on Sep 30, 2022. It is now read-only.

Conversation

@jsquyres
Copy link
Member

Don't destroy the event base before destroying all objects that contain events (and call event_del(), which uses the event base on which event is enqueued).

This matches the ordering in the Open MPI master.

@rhc54 How do you want to handle this? Do you want piecemeal PMIx bug fixes like this, or do you want to do a re-sync of PMIx from OMPI master (or PMIx master?)?

This issue was initially found by valgrind on #747 (comment), and then had its own issue opened in open-mpi/ompi#1117.

@rhc54 Please review.

Don't destroy the event base before destroying all objects that
contain events (and call event_del(), which uses the event base on
which event is enqueued).

This matches the ordering in the Open MPI master.

Fixes open-mpi/ompi#1117
@jsquyres jsquyres added the bug label Nov 10, 2015
@jsquyres jsquyres added this to the v2.0.0 milestone Nov 10, 2015
@rhc54
Copy link

rhc54 commented Nov 10, 2015

As I said on the other ticket you opened on this, it appears to be simply a case of 2.x not having all the pmix updates yet. I wouldn't piecemeal it - the plan is to provide a complete update once pmix 1.1.0 is released.

@mellanox-github
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ompi-release-pr/999/ for details.

@jsquyres
Copy link
Member Author

Per discussion on the 2015-11-10, @rhc54 says the fix is fine. The ultimate plan is to re-sync pmix in both master and v2.x when pmix 1.1 is released (at some point in the near future). So let's get this fix in.

jsquyres added a commit that referenced this pull request Nov 10, 2015
@jsquyres jsquyres merged commit c213352 into open-mpi:v2.x Nov 10, 2015
alinask pushed a commit to alinask/ompi-release that referenced this pull request Dec 10, 2015
@jsquyres jsquyres deleted the pr/v2.x/pmix-destructor-ordering-fix branch February 2, 2016 19:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants