Fix leak of comm and datatype references for mprobe/improbe and fix a request leak in improbe

This commit was SVN r28157.

ompi/mca/pml/bfo/pml_bfo_iprobe.c

@@ -90,6 +90,9 @@ mca_pml_bfo_improbe(int src,
     int rc = OMPI_SUCCESS;
     mca_pml_bfo_recv_request_t *recvreq;
 
+    *message = ompi_message_alloc();
+    if (NULL == *message) return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
+
     MCA_PML_BFO_RECV_REQUEST_ALLOC(recvreq, rc);
     if (NULL == recvreq)
         return rc;
@@ -106,19 +109,21 @@ mca_pml_bfo_improbe(int src,
         }
         *matched = 1;
 
-        *message = ompi_message_alloc();
         (*message)->comm = comm;
         (*message)->req_ptr = recvreq;
+        (*message)->peer = recvreq->req_recv.req_base.req_ompi.req_status.MPI_SOURCE;
         (*message)->count = recvreq->req_recv.req_base.req_ompi.req_status._ucount;
 
-        rc = OMPI_SUCCESS;
+        rc = recvreq->req_recv.req_base.req_ompi.req_status.MPI_ERROR;
     } else {
         *matched = 0;
 
         /* we only free if we didn't match, because we're going to
            translate the request into a receive request later on if it
            was matched */
-        ompi_request_free((ompi_request_t**)&recvreq);
+        MCA_PML_BFO_RECV_REQUEST_RETURN( recvreq );
+        ompi_message_return(*message);
+        *message = MPI_MESSAGE_NULL;
         
         opal_progress();
     }
@@ -137,6 +142,9 @@ mca_pml_bfo_mprobe(int src,
     int rc = OMPI_SUCCESS;
     mca_pml_bfo_recv_request_t *recvreq;
 
+    *message = ompi_message_alloc();
+    if (NULL == *message) return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
+
     MCA_PML_BFO_RECV_REQUEST_ALLOC(recvreq, rc);
     if (NULL == recvreq)
         return rc;
@@ -148,15 +156,16 @@ mca_pml_bfo_mprobe(int src,
     MCA_PML_BFO_RECV_REQUEST_START(recvreq);
 
     ompi_request_wait_completion(&recvreq->req_recv.req_base.req_ompi);
+    rc = recvreq->req_recv.req_base.req_ompi.req_status.MPI_ERROR;
 
     if( NULL != status ) {
         *status = recvreq->req_recv.req_base.req_ompi.req_status;
     }
 
-    *message = ompi_message_alloc();
     (*message)->comm = comm;
     (*message)->req_ptr = recvreq;
+    (*message)->peer = recvreq->req_recv.req_base.req_ompi.req_status.MPI_SOURCE;
     (*message)->count = recvreq->req_recv.req_base.req_ompi.req_status._ucount;
 
-    return OMPI_SUCCESS;
+    return rc;
 }

ompi/mca/pml/bfo/pml_bfo_irecv.c

@@ -142,11 +142,21 @@ mca_pml_bfo_imrecv( void *buf,
     seq = recvreq->req_recv.req_base.req_sequence;
 
     /* make the request a recv request again */
+    /* The old request kept pointers to comm and the char datatype.
+       We're about to release those, but need to make sure comm
+       doesn't go out of scope (we don't care about the char datatype
+       anymore).  So retain comm, then release the frag, then reinit
+       the frag (which will retain comm), then release comm (but the
+       frag still has it's ref, so it'll stay in scope).  Make
+       sense? */
+    OBJ_RETAIN(comm);
+    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_RECV;
     MCA_PML_BFO_RECV_REQUEST_INIT(recvreq,
                                   buf,
                                   count, datatype, 
                                   src, tag, comm, false);
+    OBJ_RELEASE(comm);
 
     PERUSE_TRACE_COMM_EVENT (PERUSE_COMM_REQ_ACTIVATE,
                              &((recvreq)->req_recv.req_base),
@@ -170,7 +180,7 @@ mca_pml_bfo_imrecv( void *buf,
     recvreq->req_recv.req_base.req_proc = proc->ompi_proc;
     prepare_recv_req_converter(recvreq);
 
-    /* we can't go throught he match, since we already have the match.
+    /* we can't go through the match, since we already have the match.
        Cheat and do what REQUEST_START does, but without the frag
        search */
     hdr = (mca_pml_bfo_hdr_t*)frag->segments->seg_addr.pval;
@@ -227,11 +237,21 @@ mca_pml_bfo_mrecv( void *buf,
     bfo_comm = recvreq->req_recv.req_base.req_comm->c_pml_comm;
 
     /* make the request a recv request again */
+    /* The old request kept pointers to comm and the char datatype.
+       We're about to release those, but need to make sure comm
+       doesn't go out of scope (we don't care about the char datatype
+       anymore).  So retain comm, then release the frag, then reinit
+       the frag (which will retain comm), then release comm (but the
+       frag still has it's ref, so it'll stay in scope).  Make
+       sense? */
+    OBJ_RETAIN(comm);
+    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_RECV;
     MCA_PML_BFO_RECV_REQUEST_INIT(recvreq,
                                   buf,
                                   count, datatype, 
                                   src, tag, comm, false);
+    OBJ_RELEASE(comm);
 
     PERUSE_TRACE_COMM_EVENT (PERUSE_COMM_REQ_ACTIVATE,
                              &((recvreq)->req_recv.req_base),
@@ -254,7 +274,7 @@ mca_pml_bfo_mrecv( void *buf,
     recvreq->req_recv.req_base.req_proc = proc->ompi_proc;
     prepare_recv_req_converter(recvreq);
 
-    /* we can't go throught he match, since we already have the match.
+    /* we can't go through the match, since we already have the match.
        Cheat and do what REQUEST_START does, but without the frag
        search */
     hdr = (mca_pml_bfo_hdr_t*)frag->segments->seg_addr.pval;

ompi/mca/pml/ob1/pml_ob1_iprobe.c

@@ -91,9 +91,14 @@ mca_pml_ob1_improbe(int src,
     int rc = OMPI_SUCCESS;
     mca_pml_ob1_recv_request_t *recvreq;
 
+    *message = ompi_message_alloc();
+    if (NULL == *message) return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
+
     MCA_PML_OB1_RECV_REQUEST_ALLOC(recvreq, rc);
-    if (NULL == recvreq)
+    if (NULL == recvreq) {
+        ompi_message_return(*message);
         return rc;
+    }
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_IMPROBE;
 
     /* initialize the request enough to probe and get the status */
@@ -107,20 +112,21 @@ mca_pml_ob1_improbe(int src,
         }
         *matched = 1;
 
-        *message = ompi_message_alloc();
         (*message)->comm = comm;
         (*message)->req_ptr = recvreq;
         (*message)->peer = recvreq->req_recv.req_base.req_ompi.req_status.MPI_SOURCE;
         (*message)->count = recvreq->req_recv.req_base.req_ompi.req_status._ucount;
 
-        rc = OMPI_SUCCESS;
+        rc = recvreq->req_recv.req_base.req_ompi.req_status.MPI_ERROR;
     } else {
         *matched = 0;
 
         /* we only free if we didn't match, because we're going to
            translate the request into a receive request later on if it
            was matched */
-        ompi_request_free((ompi_request_t**)&recvreq);
+        MCA_PML_OB1_RECV_REQUEST_RETURN( recvreq );
+        ompi_message_return(*message);
+        *message = MPI_MESSAGE_NULL;
         
         opal_progress();
     }
@@ -139,9 +145,14 @@ mca_pml_ob1_mprobe(int src,
     int rc = OMPI_SUCCESS;
     mca_pml_ob1_recv_request_t *recvreq;
 
+    *message = ompi_message_alloc();
+    if (NULL == *message) return OMPI_ERR_TEMP_OUT_OF_RESOURCE;
+
     MCA_PML_OB1_RECV_REQUEST_ALLOC(recvreq, rc);
-    if (NULL == recvreq)
+    if (NULL == recvreq) {
+        ompi_message_return(*message);
         return rc;
+    }
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_MPROBE;
 
     /* initialize the request enough to probe and get the status */
@@ -150,15 +161,16 @@ mca_pml_ob1_mprobe(int src,
     MCA_PML_OB1_RECV_REQUEST_START(recvreq);
 
     ompi_request_wait_completion(&recvreq->req_recv.req_base.req_ompi);
+    rc = recvreq->req_recv.req_base.req_ompi.req_status.MPI_ERROR;
 
     if( NULL != status ) {
         *status = recvreq->req_recv.req_base.req_ompi.req_status;
     }
 
-    *message = ompi_message_alloc();
     (*message)->comm = comm;
     (*message)->req_ptr = recvreq;
+    (*message)->peer = recvreq->req_recv.req_base.req_ompi.req_status.MPI_SOURCE;
     (*message)->count = recvreq->req_recv.req_base.req_ompi.req_status._ucount;
 
-    return OMPI_SUCCESS;
+    return rc;
 }

ompi/mca/pml/ob1/pml_ob1_irecv.c

@@ -143,11 +143,21 @@ mca_pml_ob1_imrecv( void *buf,
     seq = recvreq->req_recv.req_base.req_sequence;
 
     /* make the request a recv request again */
+    /* The old request kept pointers to comm and the char datatype.
+       We're about to release those, but need to make sure comm
+       doesn't go out of scope (we don't care about the char datatype
+       anymore).  So retain comm, then release the frag, then reinit
+       the frag (which will retain comm), then release comm (but the
+       frag still has it's ref, so it'll stay in scope).  Make
+       sense? */
+    OBJ_RETAIN(comm);
+    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_RECV;
     MCA_PML_OB1_RECV_REQUEST_INIT(recvreq,
                                   buf,
                                   count, datatype, 
                                   src, tag, comm, false);
+    OBJ_RELEASE(comm);
 
     PERUSE_TRACE_COMM_EVENT (PERUSE_COMM_REQ_ACTIVATE,
                              &((recvreq)->req_recv.req_base),
@@ -171,7 +181,7 @@ mca_pml_ob1_imrecv( void *buf,
     recvreq->req_recv.req_base.req_proc = proc->ompi_proc;
     prepare_recv_req_converter(recvreq);
 
-    /* we can't go throught he match, since we already have the match.
+    /* we can't go through the match, since we already have the match.
        Cheat and do what REQUEST_START does, but without the frag
        search */
     hdr = (mca_pml_ob1_hdr_t*)frag->segments->seg_addr.pval;
@@ -228,11 +238,21 @@ mca_pml_ob1_mrecv( void *buf,
     ob1_comm = recvreq->req_recv.req_base.req_comm->c_pml_comm;
 
     /* make the request a recv request again */
+    /* The old request kept pointers to comm and the char datatype.
+       We're about to release those, but need to make sure comm
+       doesn't go out of scope (we don't care about the char datatype
+       anymore).  So retain comm, then release the frag, then reinit
+       the frag (which will retain comm), then release comm (but the
+       frag still has it's ref, so it'll stay in scope).  Make
+       sense? */
+    OBJ_RETAIN(comm);
+    MCA_PML_BASE_RECV_REQUEST_FINI(&recvreq->req_recv);
     recvreq->req_recv.req_base.req_type = MCA_PML_REQUEST_RECV;
     MCA_PML_OB1_RECV_REQUEST_INIT(recvreq,
                                   buf,
                                   count, datatype, 
                                   src, tag, comm, false);
+    OBJ_RELEASE(comm);
 
     PERUSE_TRACE_COMM_EVENT (PERUSE_COMM_REQ_ACTIVATE,
                              &((recvreq)->req_recv.req_base),
@@ -255,7 +275,7 @@ mca_pml_ob1_mrecv( void *buf,
     recvreq->req_recv.req_base.req_proc = proc->ompi_proc;
     prepare_recv_req_converter(recvreq);
 
-    /* we can't go throught he match, since we already have the match.
+    /* we can't go through the match, since we already have the match.
        Cheat and do what REQUEST_START does, but without the frag
        search */
     hdr = (mca_pml_ob1_hdr_t*)frag->segments->seg_addr.pval;