Commit 7dab7cd3 authored by Halim Amer's avatar Halim Amer Committed by Kenneth Raffenetti
Browse files

Fixed several issues related to Ibsend



The implementation of Ibsend had several issues: 1) incorrect ref
counting when canceling a request, which leads to handle leaks; 2)
writing MPI_REQUEST_NULL directly in the handle field of a request
(MPID_Request) in the request pool because the handle field is passed
by reference to MPIR_Wait_impl; 3) the memory used in the attached
buffer is never freed after a successful cancellation

This patch fixes all the above mentioned issues. In addition it
fixes and closes ticket:287.
Signed-off-by: Kenneth Raffenetti's avatarKen Raffenetti <raffenet@mcs.anl.gov>
parent 95a64b67
......@@ -81,6 +81,7 @@ static int MPIR_Bsend_check_active ( void );
static MPIR_Bsend_data_t *MPIR_Bsend_find_buffer( int );
static void MPIR_Bsend_take_buffer( MPIR_Bsend_data_t *, int );
static int MPIR_Bsend_finalize( void * );
static void MPIR_Bsend_free_segment( MPIR_Bsend_data_t * );
/*
* Attach a buffer. This checks for the error conditions and then
......@@ -316,6 +317,39 @@ int MPIR_Bsend_isend(const void *buf, int count, MPI_Datatype dtype,
goto fn_exit;
}
/*
* The following routine looks up the segment used by request req
* and frees it. The request is assumed to be completed. This routine
* is called by only MPIR_Ibsend_free.
*/
#undef FUNCNAME
#define FUNCNAME MPIR_Bsend_free_seg
#undef FCNAME
#define FCNAME MPIDI_QUOTE(FUNCNAME)
int MPIR_Bsend_free_req_seg( MPID_Request* req )
{
int mpi_errno = MPI_ERR_INTERN;
MPIR_Bsend_data_t *active = BsendBuffer.active;
MPIU_DBG_MSG_P(BSEND,TYPICAL,"Checking active starting at %p", active);
while (active) {
if (active->request == req) {
MPIR_Bsend_free_segment( active );
mpi_errno = MPI_SUCCESS;
}
active = active->next;;
MPIU_DBG_MSG_P(BSEND,TYPICAL,"Next active is %p",active);
}
fn_exit:
return mpi_errno;
fn_fail:
goto fn_exit;
}
/*
* The following routines are used to manage the allocation of bsend segments
* in the user buffer. These routines handle, for example, merging segments
......
......@@ -12,4 +12,5 @@ int MPIR_Bsend_attach( void *, int );
int MPIR_Bsend_detach( void *, int * );
int MPIR_Bsend_isend(const void *, int, MPI_Datatype, int, int, MPID_Comm *,
MPIR_Bsend_kind_t, MPID_Request ** );
int MPIR_Bsend_free_req_seg(MPID_Request * );
......@@ -46,18 +46,8 @@ PMPI_LOCAL int MPIR_Ibsend_free( void *extra )
{
ibsend_req_info *ibsend_info = (ibsend_req_info *)extra;
/* Release the MPID_Request (there is still another ref pending
within the bsendutil functions) */
/* XXX DJG FIXME-MT should we be checking this? */
if (MPIU_Object_get_ref(ibsend_info->req) > 1) {
int inuse;
/* Note that this should mean that the request was
cancelled (that would have decremented the ref count)
*/
MPIR_Request_release_ref( ibsend_info->req, &inuse );
}
MPIU_Free( ibsend_info );
return MPI_SUCCESS;
}
#undef FUNCNAME
......@@ -70,6 +60,7 @@ PMPI_LOCAL int MPIR_Ibsend_cancel( void *extra, int complete )
ibsend_req_info *ibsend_info = (ibsend_req_info *)extra;
MPI_Status status;
MPID_Request *req = ibsend_info->req;
MPI_Request req_hdl = req->handle;
/* FIXME: There should be no unreferenced args! */
/* Note that this value should always be 1 because
......@@ -81,10 +72,16 @@ PMPI_LOCAL int MPIR_Ibsend_cancel( void *extra, int complete )
/* Try to cancel the underlying request */
mpi_errno = MPIR_Cancel_impl(req);
if (mpi_errno) MPIU_ERR_POP(mpi_errno);
mpi_errno = MPIR_Wait_impl( &req->handle, &status );
mpi_errno = MPIR_Wait_impl( &req_hdl, &status );
if (mpi_errno) MPIU_ERR_POP(mpi_errno);
MPIR_Test_cancelled_impl( &status, &ibsend_info->cancelled );
/* If the cancelation is successful, free the memory in the
attached buffer used by the request */
if (ibsend_info->cancelled) {
mpi_errno = MPIR_Bsend_free_req_seg(ibsend_info->req);
if (mpi_errno) MPIU_ERR_POP(mpi_errno);
}
fn_exit:
return mpi_errno;
fn_fail:
......@@ -119,8 +116,6 @@ int MPIR_Ibsend_impl(const void *buf, int count, MPI_Datatype datatype, int dest
if (mpi_errno) MPIU_ERR_POP(mpi_errno);
/* The request is immediately complete because the MPIR_Bsend_isend has
already moved the data out of the user's buffer */
MPIR_Request_add_ref( request_ptr );
/* Request count is now 2 (set to 1 in Grequest_start) */
MPIR_Grequest_complete_impl(new_request_ptr);
MPIU_OBJ_PUBLISH_HANDLE(*request, new_request_ptr->handle);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment