OMPI-5.0 error in MPI_Finalize() with user's comm_delete_attr_fn calling MPI

[jczhang07]

I met a problem in testing PETSc' error handler with OpenMPI-5.0. I used one MPI rank. When PETSc detected an error (integer multiplication overflow), it would print the stack trace and then call MPI_Finalize().

In program execution, petsc dupped an inner communicator from the outer MPI_COMM_WORLD. We linked the two communicators via attributes on them and set up the comm_delete_attr_fn's on them.

I found in MPI_Finalize(), ompi tried to delete attributes on MPI_COMM_WORLD, which invoked Petsc_InnerComm_Attr_Delete_Fn(). There, we had the attribute value (pointer to the inner communicator), and called some MPI functions, such as MPI_Comm_get_attr(), MPI_Comm_delete_attr() to unlink the outer comm from the inner comm.
But at that time, ompi_instance_count is already 0, MPI_Comm_get_attr just failed on checking OMPI_ERR_INIT_FINALIZE and dumped a misleading message such as

[frog:00000] *** An error occurred in MPI_Comm_get_attr
[frog:00000] *** reported by process [3339452416,0]
[frog:00000] *** on a NULL communicator
[frog:00000] *** Unknown error
[frog:00000] *** MPI_ERRORS_ARE_FATAL (processes in this communicator will now abort,
[frog:00000] ***    and MPI will try to terminate your MPI job as well)

This is the backtrace in gdb of my code, which failed at line 400.

(gdb) l
395         union
396         {
397           MPI_Comm comm;
398           void    *ptr;
399         } ocomm;
400         PetscCallMPI(MPI_Comm_get_attr(icomm.comm, Petsc_OuterComm_keyval, &ocomm, &flg));
401         if (!flg) SETERRMPI(PETSC_COMM_SELF, PETSC_ERR_ARG_CORRUPT, "Inner comm does not have OuterComm attribute");
402         if (ocomm.comm != comm) SETERRMPI(PETSC_COMM_SELF, PETSC_ERR_ARG_CORRUPT, "Inner comm's OuterComm attribute does not point to outer PETSc comm");
403       }
404       PetscCallMPI(MPI_Comm_delete_attr(icomm.comm, Petsc_OuterComm_keyval));
gdb) bt
#0  Petsc_InnerComm_Attr_Delete_Fn (comm=0x7fffb5df81a0 <ompi_mpi_comm_world>, keyval=14, attr_val=0xa37ee0, extra_state=0x0) at /home/jczhang/petsc/src/sys/objects/pinit.c:400
#1  0x00007fffb5a5b239 in ompi_attr_delete_impl (type=COMM_ATTR, object=0x7fffb5df81a0 <ompi_mpi_comm_world>, attr_hash=0xb12470, key=14, predefined=true) at attribute/attribute.c:1139
#2  0x00007fffb5a5b81d in ompi_attr_delete_all (type=COMM_ATTR, object=0x7fffb5df81a0 <ompi_mpi_comm_world>, attr_hash=0xb12470) at attribute/attribute.c:1243
#3  0x00007fffb5a5d616 in ompi_comm_finalize () at communicator/comm_init.c:319
#4  0x00007fffe7136b3d in opal_finalize_cleanup_domain (domain=0x7fffe71fade0 <opal_init_domain>) at runtime/opal_finalize_core.c:128
#5  0x00007fffe7129c08 in opal_finalize () at runtime/opal_finalize.c:56
#6  0x00007fffb5a92e0b in ompi_rte_finalize () at runtime/ompi_rte.c:1041
#7  0x00007fffb5a9aec3 in ompi_mpi_instance_finalize_common () at instance/instance.c:911
#8  0x00007fffb5a9b0c9 in ompi_mpi_instance_finalize (instance=0x7fffb5e092c0 <ompi_mpi_instance_default>) at instance/instance.c:965
#9  0x00007fffb5a8de27 in ompi_mpi_finalize () at runtime/ompi_mpi_finalize.c:294
#10 0x00007fffb5acf918 in PMPI_Finalize () at finalize.c:52
#11 0x00007ffff196f2c5 in PetscError (comm=0x7fffb5df83a0 <ompi_mpi_comm_self>, line=109, func=0x40b167 <__func__.4> "main", file=0x40b051 "ex19.c", n=PETSC_ERR_SUP, p=PETSC_ERROR_REPEAT, mess=0x40b058 " ")
    at /home/jczhang/petsc/src/sys/error/err.c:417
#12 0x0000000000402946 in main (argc=5, argv=0x7fffffffd7a8) at ex19.c:109

I checked mpi manual and it does not say users should not call MPI in callbacks.

[hppritcha]

You don't by chance have a small test case that illustrates the problem? Just being a bit lazy...

[jczhang07]

You don't by chance have a small test case that illustrates the problem? Just being a bit lazy...

I will try to provide one.

[jczhang07]

@hppritcha I added a test code.

With ompi-4.1.3

$ ./ex0
Removing the link to user's MPI_Comm
Removing the link to petsc's inner MPI_Comm

With ompi-5.0.0

[$ ./ex0
[frog:00000] *** An error occurred in MPI_Comm_get_attr
[frog:00000] *** reported by process [414580736,0]
[frog:00000] *** on a NULL communicator
[frog:00000] *** Unknown error
[frog:00000] *** MPI_ERRORS_ARE_FATAL (processes in this communicator will now abort,
[frog:00000] ***    and MPI will try to terminate your MPI job as well)]

The test code ex0. c is

#include <mpi.h>
#include <stdio.h>

typedef union
{
  MPI_Comm comm;
  void    *ptr;
} ucomm_t;

int Petsc_InnerComm_keyval, Petsc_OuterComm_keyval;

// on an outer comm, remove its innerComm attribute
int Petsc_InnerComm_Attr_Delete_Fn(MPI_Comm comm, int keyval, void *attr_val, void *extra_state)
{
  ucomm_t icomm, ocomm;
  int     flg;

  icomm.ptr = attr_val;
  MPI_Comm_get_attr(icomm.comm, Petsc_OuterComm_keyval, &ocomm, &flg);
  if (!flg) printf("Inner comm does not have an OuterComm attribute");
  MPI_Comm_delete_attr(icomm.comm, Petsc_OuterComm_keyval);
  printf("Removing the link to petsc's inner MPI_Comm\n");
  return 0;
}

// on an inner comm, remove its outerComm attribute
int Petsc_OuterComm_Attr_Delete_Fn(MPI_Comm comm, int keyval, void *attr_val, void *extra_state)
{
  printf("Removing the link to user's MPI_Comm\n");
  return 0;
}

int main(int argc, char *argv[])
{
  MPI_Comm innerComm, outerComm;
  ucomm_t  ucomm;
  int      petsc_error = 1;
  int      flg;

  MPI_Init(&argc, &argv);
  MPI_Comm_create_keyval(MPI_COMM_NULL_COPY_FN, Petsc_InnerComm_Attr_Delete_Fn, &Petsc_InnerComm_keyval, (void *)0);
  MPI_Comm_create_keyval(MPI_COMM_NULL_COPY_FN, Petsc_OuterComm_Attr_Delete_Fn, &Petsc_OuterComm_keyval, (void *)0);

  outerComm = MPI_COMM_WORLD;
  MPI_Comm_dup(outerComm, &innerComm);

  // build the outerComm-->innerComm link
  ucomm.comm = innerComm;
  MPI_Comm_set_attr(outerComm, Petsc_InnerComm_keyval, ucomm.ptr);

  // build the innerComm-->outerComm link
  ucomm.comm = outerComm;
  MPI_Comm_set_attr(innerComm, Petsc_OuterComm_keyval, ucomm.ptr);

  if (petsc_error) { // error handler code path
    MPI_Finalize();
  } else { // normal code path
    // get the outerComm linked with innerComm
    MPI_Comm_get_attr(innerComm, Petsc_OuterComm_keyval, &ucomm, &flg);
    // cut the outerComm-->innerComm link
    MPI_Comm_delete_attr(ucomm.comm, Petsc_InnerComm_keyval);
    // destroy innerComm
    MPI_Comm_free(&innerComm);
    MPI_Finalize();
  }

  return 0;
}

[jsquyres]

@hppritcha It looks like the MPI_Comm_get_attr() in the call to the Petsc_InnerComm_Attr_Delete_Fn() during MPI_Finalize() is failing the error check OMPI_ERR_INIT_FINALIZE(FUNC_NAME).

Is the "MPI is finalized" flag being set to soon?

[jsquyres]

@hppritcha I dug a little deeper and I think there's multiple issues here:

1. The error message emitted by OMPI_ERR_INIT_FINALIZE() could be better. As @jczhang07 stated, the message isn't helpful -- it doesn't say "Hey, you invoked a function after MPI_Finalize".
2. We're trying to delete attributes on MPI objects during MPI_Finalize. This seems to be wrong. According to MPI-4.1:7.7.2 p365:23-25:

The comm_delete_attr_fn function is invoked when a communicator is deleted by MPI_COMM_FREE, MPI_COMM_DISCONNECT or when a call is made explicitly to MPI_COMM_DELETE_ATTR.

Meaning: we should not be deleting attributes during MPI_Finalize (except for those on MPI_COMM_SELF).

Right?

[jczhang07]

@jsquyres You imply users should delete any attributes attached to MPI_COMM_WORLD before MPI_Finalize() and should not expect comm_delete_attr_fn be finally called by MPI_Finalize() to do clean-up work?

[jsquyres]

What I'm saying is that the standard says that the delete function is only invoked:

1. For attributes on MPI_COMM_SELF during MPI_FINALIZE.
2. Or during calls to MPI_COMM_FREE, MPI_COMM_DISCONNECT, or MPI_COMM_DELETE_ATTR.

At least in MPI-4.1:7.7.2, it does not say that the delete function is invoked for attributes on other communicators during MPI_FINALIZE. Does it say that somewhere else?

[jczhang07]

No. I could not find it. I think MPI standard should clarify when attributes on MPI_COMM_WORLD are deleted.

[jsquyres]

I don't think that implementations are required to delete attributes on MPI_COMM_WORLD at all.

Is there a reason you're not caching your attributes on MPI_COMM_SELF? Because those are guaranteed to be deleted right at the beginning of MPI_FINALIZE.

[jczhang07]

We use attributes to stash the internal communicator (innerComm) petsc uses corresponding to a user communicator (outerComm). The outerComm can be any communicator, say, MPI_COMM_WORLD. So that we can have code like this

MatCreate(outerComm, &A); // for the first time, petsc creates innerComm and stashes it on outerComm
MatCreate(outerComm, &B); // petsc will retrieve innerComm and reuse it to create matrix B

[jsquyres]

I think that if you want to clean up all the petsc communicators and resources during MPI_FINALIZE, you should put an attribute on MPI_COMM_SELF that gets a callback during MPI_FINALIZE, and you can have that attribute value point to some internal data structure in petsc that points to all the resources you want to clean up.

To be clear: two things happen right near the beginning of Open MPI's MPI_FINALIZE:

1. All attributes on MPI_COMM_SELF are destroyed (and their callbacks are invoked)
2. The world model is marked as finalized, such that subsequent calls to other MPI APIs will fail because MPI has been finalized

This bug is that Open MPI is still going through and invoking the callbacks on other communicators, but after the MPI world model is marked as "finalized", so your calls to things like MPI_COMM_GET_ATTR are resulting in an (unfortunately poorly labeled) error. I suspect that the fix will be that we just won't invoke the attribute delete callbacks in this case, but @hppritcha would need to provide some more analysis here.

Regardless, I don't think an MPI implementation is obligated to invoke attribute delete callbacks during MPI_FINALIZE on anything other than MPI_COMM_SELF. If Open MPI and/or MPICH have been invoking attribute delete callbacks on MPI objects other than MPI_COMM_SELF during MPI_FINALIZE in prior versions, then I think you've been getting lucky, because this behavior does not appear to be required by the standard. Indeed, I think part of the reason why it's not mandated is because there's inherent causality and ordering issues if non-MPI_COMM_SELF attributes are to be deleted during MPI_FINALIZE. E.g., what if Open MPI deletes all communicator attributes and then destroys all communicators, and then goes to delete all window attributes. What if one of the window attribute delete callbacks makes an MPI API call that involves a communicator -- possibly even MPI_COMM_SELF or MPI_COMM_WORLD? That's very much not defined in the standard -- it's an entire can of worms that has not been opened.

I believe that there have been prior presentations to the Forum about these kinds of causality / ordering issues (from Fab Tillier and myself, several years ago at this point). Ah yes, I found 2 presentations from back in 2013:

• Initial presentation to the MPI Forum by Fab Tillier about some of the problems with how attributes are defined: https://github.com/mpi-forum/mpi-forum.github.io/blob/master/_slides/2013/06/MPI_Keyval.pdf
• Followup presentation at the next meeting by me + Fab about a proposal for a new, well-defined attribute system: https://github.com/mpi-forum/mpi-forum.github.io/blob/master/_slides/2013/09/jsquyres-attributes-revamp.pdf

Fab and I were not able to get enough interest to follow through on the "new attribute" system, so the attributes definitions in the current standard has many of the same issues discussed in that first presentation.

[jczhang07]

@jsquyres

Regardless, I don't think an MPI implementation is obligated to invoke attribute delete callbacks during MPI_FINALIZE on anything other than MPI_COMM_SELF.

I agree. A correct PETSc program does not need MPI_FINALIZE to free attributes set by petsc on MPI_COMM_WORLD. PETSc deletes them before MPI_FINALIZE.

The problem here is OpenMPI does delete attributes on MPI_COMM_WORLD and give a misleading error message.

I think for OpenMPI the easiest fix is just to not delete attributes on MPI_COMM_WORLD

[hppritcha]

@hppritcha I dug a little deeper and I think there's multiple issues here:

1. The error message emitted by OMPI_ERR_INIT_FINALIZE() could be better. As @jczhang07 stated, the message isn't helpful -- it doesn't say "Hey, you invoked a function after MPI_Finalize".
2. We're trying to delete attributes on MPI objects during MPI_Finalize. This seems to be wrong. According to MPI-4.1:7.7.2 p365:23-25:

The comm_delete_attr_fn function is invoked when a communicator is deleted by MPI_COMM_FREE, MPI_COMM_DISCONNECT or when a call is made explicitly to MPI_COMM_DELETE_ATTR.

Meaning: we should not be deleting attributes during MPI_Finalize (except for those on MPI_COMM_SELF).

Right?

I think your analysis is correct @jsquyres . I'll fix these two bugs (certainly 2, not sure about how easy 1 will be).