From 17ec412c5743fff3f4dfeef203e101ec55328a7c Mon Sep 17 00:00:00 2001 From: Jeff Squyres Date: Sat, 26 Mar 2005 01:49:14 +0000 Subject: [PATCH] Add another flag to the communicator: PML_ADDED. This flag is only set after we call mca_pml.pml_comm_add(). This allows us, in the destructor, to only call mca_pml.pml_comm_del() if the corresponding pml_comm_add() was called. In this way, if an error occurs during the construction of a communicator (e.g., attributes failing to copy, as in the Intel test MPI_Keyval3_c), we simply propagate the error up the stack so that an MPI exception can be generated -- not cause a failed assert() in teg's pml_del_comm(). This commit was SVN r5032. --- src/communicator/comm.c | 2 ++ src/communicator/comm_init.c | 15 +++++++++++++-- src/communicator/communicator.h | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/communicator/comm.c b/src/communicator/comm.c index cbcaf7958b..d3fdf9c59d 100644 --- a/src/communicator/comm.c +++ b/src/communicator/comm.c @@ -189,6 +189,7 @@ int ompi_comm_set ( ompi_communicator_t *newcomm, OBJ_RELEASE(newcomm); return OMPI_ERROR; } + OMPI_COMM_SET_PML_ADDED(newcomm); return (OMPI_SUCCESS); } @@ -1329,6 +1330,7 @@ static int ompi_comm_fill_rest (ompi_communicator_t *comm, /* some error has happened */ return ret; } + OMPI_COMM_SET_PML_ADDED(comm); return OMPI_SUCCESS; } diff --git a/src/communicator/comm_init.c b/src/communicator/comm_init.c index af97196df5..00761e34a5 100644 --- a/src/communicator/comm_init.c +++ b/src/communicator/comm_init.c @@ -85,6 +85,7 @@ int ompi_comm_init(void) ompi_mpi_comm_world.error_handler = &ompi_mpi_errors_are_fatal; OBJ_RETAIN( &ompi_mpi_errors_are_fatal ); mca_pml.pml_add_comm(&ompi_mpi_comm_world); + OMPI_COMM_SET_PML_ADDED(&ompi_mpi_comm_world); ompi_pointer_array_set_item (&ompi_mpi_communicators, 0, &ompi_mpi_comm_world); strncpy (ompi_mpi_comm_world.c_name, "MPI_COMM_WORLD", @@ -114,6 +115,7 @@ int ompi_comm_init(void) ompi_mpi_comm_self.error_handler = &ompi_mpi_errors_are_fatal; OBJ_RETAIN( &ompi_mpi_errors_are_fatal ); mca_pml.pml_add_comm(&ompi_mpi_comm_self); + OMPI_COMM_SET_PML_ADDED(&ompi_mpi_comm_self); ompi_pointer_array_set_item (&ompi_mpi_communicators, 1, &ompi_mpi_comm_self); strncpy(ompi_mpi_comm_self.c_name,"MPI_COMM_SELF",strlen("MPI_COMM_SELF")+1); @@ -331,9 +333,18 @@ static void ompi_comm_destruct(ompi_communicator_t* comm) mca_pml.pml_add_comm() was called explicitly in ompi_comm_init() when setting up COMM_WORLD and COMM_SELF; it's called in ompi_comm_set() for all others. This means that all - communicators must be destroyed before the PML shuts down. */ + communicators must be destroyed before the PML shuts down. - if ( MPI_COMM_NULL != comm ) { + Also -- do not invoke the pml_del_comm if the corresponding + pml_add_comm was never invoked. This can happen in an error + situation where, for example, attributes do not copy properly + from one communicator to another and we end up destroying the + new communication while propagating the error up the stack. We + want to make it all the way up the stack to invoke the MPI + exception, not cause a seg fault in pml_del_comm because it was + never pml_add_com'ed. */ + + if ( MPI_COMM_NULL != comm && OMPI_COMM_IS_PML_ADDED(comm) ) { mca_pml.pml_del_comm (comm); } diff --git a/src/communicator/communicator.h b/src/communicator/communicator.h index 8ad155b26e..8618fea45b 100644 --- a/src/communicator/communicator.h +++ b/src/communicator/communicator.h @@ -45,6 +45,7 @@ OMPI_DECLSPEC extern ompi_class_t ompi_communicator_t_class; #define OMPI_COMM_HIDDEN 0x00000040 #define OMPI_COMM_DYNAMIC 0x00000080 #define OMPI_COMM_INVALID 0x00000100 +#define OMPI_COMM_PML_ADDED 0x00000200 /* some utility #defines */ #define OMPI_COMM_IS_INTER(comm) ((comm)->c_flags & OMPI_COMM_INTER) @@ -56,11 +57,14 @@ OMPI_DECLSPEC extern ompi_class_t ompi_communicator_t_class; #define OMPI_COMM_IS_FREED(comm) ((comm)->c_flags & OMPI_COMM_ISFREED) #define OMPI_COMM_IS_DYNAMIC(comm) ((comm)->c_flags &OMPI_COMM_DYNAMIC) #define OMPI_COMM_IS_INVALID(comm) ((comm)->c_flags &OMPI_COMM_INVALID) +#define OMPI_COMM_IS_PML_ADDED(comm) ((comm)->c_flags &OMPI_COMM_PML_ADDED) #define OMPI_COMM_SET_HIDDEN(comm) ((comm)->c_flags |= OMPI_COMM_HIDDEN) #define OMPI_COMM_SET_DYNAMIC(comm) ((comm)->c_flags |= OMPI_COMM_DYNAMIC) #define OMPI_COMM_SET_INVALID(comm) ((comm)->c_flags |= OMPI_COMM_INVALID) +#define OMPI_COMM_SET_PML_ADDED(comm) ((comm)->c_flags |= OMPI_COMM_PML_ADDED) + /* a set of special tags: */ /* to recognize an MPI_Comm_join in the comm_connect_accept routine. */