[petsc-dev] threadcomm memory leak

[Shri]

Barry, Jed,
Please see the attached patch based on Barry's suggestions. I tested this with MPI and MPIUNI and did not see any memory leaks. Let me know what you think.

Thanks,
Shri
-------------- next part --------------
A non-text attachment was scrubbed...
Name: threadcomm.patch
Type: application/octet-stream
Size: 7298 bytes
Desc: not available
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120717/d2b22cd3/attachment.obj>
-------------- next part --------------



On Jul 16, 2012, at 11:00 PM, Barry Smith wrote:

> 
>  Note that in general I would advocate in any code (especially PETSc code) NEVER blinding putting an attribute into a MPI_Comm, always check if the attribute is already there and only put it there if it is not already there.
> 
>    Barry
> 
> On Jul 16, 2012, at 10:48 PM, Jed Brown wrote:
> 
>> On Mon, Jul 16, 2012 at 10:44 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
>>  /* PETSC_COMM_SELF = PETSC_COMM_WORLD for MPIUNI */
>> #if !defined(PETSC_HAVE_MPIUNI)
>>  ierr = PetscCommDuplicate(PETSC_COMM_WORLD,&icomm,PETSC_NULL);CHKERRQ(ierr);
>>  ierr = MPI_Attr_put(icomm,Petsc_ThreadComm_keyval,(void*)tcomm);CHKERRQ(ierr);
>>  tcomm->refct++;               /* Share the threadcomm with PETSC_COMM_SELF */
>> #endif
>> 
>>  ierr = PetscCommDuplicate(PETSC_COMM_SELF,&icomm,PETSC_NULL);CHKERRQ(ierr);
>>  ierr = MPI_Attr_put(icomm,Petsc_ThreadComm_keyval,(void*)tcomm);CHKERRQ(ierr);
>> 
>> 
>>  I would not do it this way. Instead I would write a general routine that attached a threadcomm to a MPI_Comm; this routine would get the threadcomm_keyval and if it did NOT find it then would be put the attribute, otherwise it would know one was already there. Say it is called PetscThreadCommAttach(MPI_Comm, threadcomm); then in this routine you would just write
>> 
>>    PetscThreadCommAttach(PETSC_COMM_WORLD, tcomm);
>>    PetscThreadCommAttach(PETSC_COMM_SELF,tcomm);        /* won't attr it again for MPIUni because it is already there */
>> 
>> This looks good, but there is also a ref-counting check needed in PetscThreadCommDetach/Destroy because the thread pool (presumably) needs to be closed before PetscFinalize returns.
>