On Apr 5, 2013, at 8:28 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
> [Cc'ing Lisandro because petsc4py seems to be most sensitive to changes like > this, though I don't think this affects him.] > > > On Fri, Apr 5, 2013 at 8:01 PM, Barry Smith <bsmith at mcs.anl.gov> wrote: > Currently PetscFunctionListAdd() maintains a list of all function lists in > existence and in PetscFinalize() PetscFunctionListDestroyAll() loops over all > the list of lists destroying each list. Thus all of these lists are destroyed > without the XXXRegisterDestroy() needing to be called. A drawback is that > PetscFunctionListDestroyAll() is called before some of the > XXXFinalizePackage() are called thus leaving XXXList global variables > pointing to freed memory (this is why for example VecFinalizePackage() has > VecList = NULL; in it.) > > I propose the following change; each appropriate XXXFinalizePackage() call > the appropriate XXXRegisterDestroy() (this would replace the VecList > = NULL; lines) and PetscFunctionListDestroyAll() be renamed to just > generate a dump of any lists that don't get properly freed. > > Okay, note that PetscFunctionListAdd could store the address of the > FunctionList (which it is passed) rather than just the list, in which case > PetscFunctionListDestroyAll() would be able to zero VecList itself, getting > rid of the problem of dangling lists. > > I think we should either do what you propose and consistently make > VecList/MatList/etc static or we should consistently make them visible, get > rid of VecRegisterAllCalled (which is redundant if > PetscFunctionListDestroyAll can zero VecList by storing the pointer), and do > away with all the XRegisterDestroy functions. > > I may be wrong, but I think it's rare that a user really wants to selectively > clear a FunctionList, and in the rare case that they need it, > PetscFunctionListDestroy(&VecList) would do the trick. I'm learn towards not having a master "destroy all lists" approach (the current model) and just having each XXXFinalizePackage() call PetscFunctionListDestroy(&appropriate) so the XRegisterDestroy() are gone.
