Merge pull request #9296 from abouteiller/bugfix/sendrecv-err

Prevent deadlock after an unmanaged error in MPI_SENDRECV

Author: hppritcha

ompi/mca/pml/ob1/pml_ob1_isend.c

@@ -206,7 +206,6 @@ int mca_pml_ob1_isend(const void *buf,
 
 #if OPAL_ENABLE_FT_MPI
 alloc_ft_req:
-#endif /* OPAL_ENABLE_FT_MPI */
     MCA_PML_OB1_SEND_REQUEST_ALLOC(comm, dst, sendreq);
     if (NULL == sendreq)
         return OMPI_ERR_OUT_OF_RESOURCE;
@@ -224,10 +223,12 @@ int mca_pml_ob1_isend(const void *buf,
 
     /* No point in starting the request, it won't go through, mark completed
      * in error for collection in future wait */
-    sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR = MPI_ERR_PROC_FAILED;
+    sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR = ompi_comm_is_revoked(comm)? MPI_ERR_REVOKED: MPI_ERR_PROC_FAILED;
     MCA_PML_OB1_SEND_REQUEST_MPI_COMPLETE(sendreq, false);
+    OPAL_OUTPUT_VERBOSE((2, "Allocating request in error %s (peer %d, seq %d) with error code %d", sendreq, dst, sendreq->req_send.req_base.req_sequence, sendreq->req_send.req_base.req_ompi.req_status.MPI_ERROR));
     *request = (ompi_request_t *) sendreq;
     return OMPI_SUCCESS;
+#endif /* OPAL_ENABLE_FT_MPI */
 }
 
 int mca_pml_ob1_send(const void *buf,

ompi/mca/pml/ob1/pml_ob1_recvfrag.c

@@ -419,7 +419,7 @@ int mca_pml_ob1_revoke_comm( struct ompi_communicator_t* ompi_comm, bool coll_on
             assert( MCA_PML_OB1_HDR_TYPE_RGET == hdr->hdr_common.hdr_type ||
                     MCA_PML_OB1_HDR_TYPE_RNDV == hdr->hdr_common.hdr_type );
             OPAL_OUTPUT_VERBOSE((2, ompi_ftmpi_output_handle,
-                                 "ob1_revoke_comm: sending NACK to %d", hdr->hdr_rndv.hdr_match.hdr_src));
+                                 "ob1_revoke_comm: sending NACK to %d for seq %d", hdr->hdr_rndv.hdr_match.hdr_src, hdr->hdr_rndv.hdr_match.hdr_seq));
             /* Send a ACK with a NULL request to signify revocation */
             proc = mca_pml_ob1_peer_lookup(ompi_comm, hdr->hdr_rndv.hdr_match.hdr_src);
             mca_pml_ob1_recv_request_ack_send(NULL, proc->ompi_proc, hdr->hdr_rndv.hdr_src_req.lval, NULL, 0, 0, false);
@@ -428,7 +428,7 @@ int mca_pml_ob1_revoke_comm( struct ompi_communicator_t* ompi_comm, bool coll_on
             /* if it's a TYPE_MATCH, the sender is not expecting anything
              * from us. So we are done. */
             OPAL_OUTPUT_VERBOSE((15, ompi_ftmpi_output_handle,
-                                 "ob1_revoke_comm: dropping silently frag from %d", hdr->hdr_rndv.hdr_match.hdr_src));
+                                 "ob1_revoke_comm: dropping silently frag from %d for seq %d", hdr->hdr_rndv.hdr_match.hdr_src, hdr->hdr_rndv.hdr_match.hdr_seq));
         }
         MCA_PML_OB1_RECV_FRAG_RETURN(frag);
     }
@@ -681,7 +681,7 @@ void mca_pml_ob1_recv_frag_callback_ack (mca_btl_base_module_t *btl,
 #if OPAL_ENABLE_FT_MPI
     /* if the req_recv is NULL, the comm has been revoked at the receiver */
     if( OPAL_UNLIKELY(NULL == sendreq->req_recv.pval) ) {
-        OPAL_OUTPUT_VERBOSE((2, ompi_ftmpi_output_handle, "Recvfrag: Received a NACK to the RDV/RGET match to %d on comm %d\n", sendreq->req_send.req_base.req_peer, sendreq->req_send.req_base.req_comm->c_contextid));
+        OPAL_OUTPUT_VERBOSE((2, ompi_ftmpi_output_handle, "Recvfrag: Received a NACK to the RDV/RGET match to %d for seq %d on comm %d\n", sendreq->req_send.req_base.req_peer, sendreq->req_send.req_base.req_sequence, sendreq->req_send.req_base.req_comm->c_contextid));
         if (NULL != sendreq->rdma_frag) {
             MCA_PML_OB1_RDMA_FRAG_RETURN(sendreq->rdma_frag);
             sendreq->rdma_frag = NULL;
@@ -1074,7 +1074,7 @@ static int mca_pml_ob1_recv_frag_match (mca_btl_base_module_t *btl,
             /* Send a ACK with a NULL request to signify revocation */
             mca_pml_ob1_rendezvous_hdr_t* hdr_rndv = (mca_pml_ob1_rendezvous_hdr_t*) hdr;
             mca_pml_ob1_recv_request_ack_send(NULL, proc->ompi_proc, hdr_rndv->hdr_src_req.lval, NULL, 0, 0, false);
-            OPAL_OUTPUT_VERBOSE((2, ompi_ftmpi_output_handle, "Recvfrag: comm %d is revoked or collectives force errors, sending a NACK to the RDV/RGET match from %d\n", hdr->hdr_ctx, hdr->hdr_src));
+            OPAL_OUTPUT_VERBOSE((2, ompi_ftmpi_output_handle, "Recvfrag: comm %d is revoked or collectives force errors, sending a NACK to the RDV/RGET match from %d for seq %d\n", hdr->hdr_ctx, hdr->hdr_src, hdr->hdr_seq));
         }
         else {
             OPAL_OUTPUT_VERBOSE((15, ompi_ftmpi_output_handle,

ompi/mpi/c/sendrecv.c

@@ -92,21 +92,27 @@ int MPI_Sendrecv(const void *sendbuf, int sendcount, MPI_Datatype sendtype,
     if (dest != MPI_PROC_NULL) { /* send */
         rc = MCA_PML_CALL(send(sendbuf, sendcount, sendtype, dest,
                                sendtag, MCA_PML_BASE_SEND_STANDARD, comm));
+        if (OPAL_UNLIKELY(MPI_SUCCESS != rc)) {
+            rcs = rc;
 #if OPAL_ENABLE_FT_MPI
-        /* If ULFM is enabled we need to wait for the posted receive to
-         * complete, hence we cannot return here */
-        rcs = rc;
-#else
-        OMPI_ERRHANDLER_CHECK(rc, comm, rc, FUNC_NAME);
-#endif  /* OPAL_ENABLE_FT_MPI */
+            /* If this is a PROC_FAILED error, we still need to proceed with
+             * the receive, so that we do not propagate errors to the sender in
+             * the case src != dst, and only dst is dead. In this case the 
+             * recv is garanteed to complete (either in error if the source is 
+             * dead, or successfully if the source is live). */
+            if (OPAL_UNLIKELY(MPI_ERR_PROC_FAILED != rc))
+            /* if intentionally spills outside ifdef */
+#endif
+            ompi_request_cancel(req);
+        }
     }
 
     if (source != MPI_PROC_NULL) { /* wait for recv */
         rc = ompi_request_wait(&req, status);
 #if OPAL_ENABLE_FT_MPI
         /* Sendrecv never returns ERR_PROC_FAILED_PENDING because it is
-         * blocking. Lets complete now that irecv and promote the error
-         * to ERR_PROC_FAILED */
+         * blocking. Lets cancel that irecv to complete it NOW and promote
+         * the error to ERR_PROC_FAILED */
         if( OPAL_UNLIKELY(MPI_ERR_PROC_FAILED_PENDING == rc) ) {
             ompi_request_cancel(req);
             ompi_request_wait(&req, MPI_STATUS_IGNORE);
@@ -119,11 +125,9 @@ int MPI_Sendrecv(const void *sendbuf, int sendcount, MPI_Datatype sendtype,
         }
         rc = MPI_SUCCESS;
     }
-#if OPAL_ENABLE_FT_MPI
     if( OPAL_UNLIKELY(MPI_SUCCESS != rcs && MPI_SUCCESS == rc) ) {
         rc = rcs;
     }
-#endif
 
     OMPI_ERRHANDLER_RETURN(rc, comm, rc, FUNC_NAME);
 }