On Thu, Mar 29, 2012 at 8:28 AM, Jed Brown <[email protected]> wrote:

On Thu, Mar 29, 2012 at 07:16, Dmitry Karpeev <[email protected]> wrote:

...

Here the problem isn't in needing to call PCReset() twice per se. The issue is that if the user uses the same PC object in two different KSPs and destroys one ksp, that will cause a PCReset() and invalidate the pc for the other ksp -- all despite increfing the pc.

Yeah, this is tricky. In this case, is it acceptable to break the dependency explicitly using

PCSetType(ksp,PCNONE); // will decrement the reference count, but not PCReset() KSPDestroy(&ksp);

Skipping the reset means that KSPReset() is basically always incorrect if the user created the PC (for example). I think a top-level KSPReset() should always reset the PC.

I am not sure how to solve the problem of the relationship between KSPReset() and PCReset() when the pc is being shared. How useful are KSPReset() and PCReset() anyway? All but one uses I found were "trivial" -- called from a PCReset_XXX, where XXX uses a KSP internally. The only nontrivial use I found was in VIRS, where the KSP and PC are reset when the active set changes. Would a simple KSPDestroy() do here instead? Perhaps there are uses of KSPReset()/PCReset() in user code, though.

Now maybe ksp->pc should be withheld by KSPDestroy() before calling KSPReset(); if the PC refct hits 0, it will be destroyed in the subsequent PCDestroy().

The present problem seems to come from a misuse of KSPReset()/PCDestroy() as a partial destructor inside KSPDestroy(): the intent here is to destroy the ksp and the underlying pc, and in the case of the pc this should be protected by the refcount. However, PCDestroy() is being entered "from a side" for the intended meaning -- via KSPReset(), not PCDestroy() and the refcount check isn't being triggered. One solution would be not to use KSPReset()/PCReset() as a partial destructor, but that would require duplicating that code directly in KSPDestroy()/PCDestroy(), or we could move this code to KSPReset_Private() that would not cascade to PCReset_Private(). Then we could have: PetscErrorCode KSPReset(KSP ksp) { ... KSPReset_Private(ksp); PCReset_Private(ksp->pc); ... } PetscErrorCode KSPDestroy(KSP *ksp) { ... KSPReset_Private(*ksp); PCDestroy((*ksp)->pc); ... } PetscErrorCode PCDestroy(PC *pc) { if (--((PetscObject)(*pc))->refct > 0) {*pc = 0; PetscFunctionReturn(0);} ... } Dmitry.