On Apr 5, 2013, at 8:28 PM, Jed Brown <[email protected]> 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 <[email protected]> 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.