Re: Fixing order of resowner cleanup in 12, for Windows

Thomas Munro <thomas.munro@gmail.com> writes:
> On Mon, May 6, 2019 at 11:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... You're not doing future
>> hackers any service by failing to include a comment that explains that
>> DSM detach MUST BE LAST, and explaining why.

> Ok, here's a version that provides a specific reason (the Windows file
> handle thing) and also a more general reasoning: we don't really want
> extension (or core) authors writing callbacks that depend on eg pins
> or locks or whatever else being still held when they run, because
> that's fragile, so calling them last is the best and most conservative
> choice.

LGTM.

> ... I think if someone does come with legitimate reasons to want
> that, we should discuss it then, and perhaps consider something a bit
> like the ResourceRelease_callbacks list: its callbacks are invoked for
> each phase.

Hmm, now that you mention it: this bit at the very end

    /* Let add-on modules get a chance too */
    for (item = ResourceRelease_callbacks; item; item = item->next)
        item->callback(phase, isCommit, isTopLevel, item->arg);

seems kind of misplaced given this discussion.  Should we not run that
*first*, before we release core resources for the same phase?  It's
a lot more plausible that extension resources depend on core resources
than vice versa.

            regards, tom lane