Re: Bogus cleanup code in PostgresNode.pm

Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> On Mon, Apr 25, 2016 at 11:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I believe we can fix this by forcing postmaster shutdown in an END
>> routine instead of a DESTROY routine, and hence propose the attached
>> patch, which does things in the right order for me. I'm a pretty
>> poor Perl programmer, so I'd appreciate somebody vetting this.

> Another, perhaps more solid approach, would be put the DESTROY method
> in charge of removing PGDATA and extend TestLib::tempdir with an
> argument to be able to switch to CLEANUP => 0 at will. Then we use
> this argument for PGDATA after sending SIGQUIT.

Bearing in mind that I'm not a Perl expert --- this bothers me because
of what I read about the order of global destructor calls being
unspecified. See http://perldoc.perl.org/perlobj.html#Destructors
specifically:

When the last reference to an object goes away, the object is
destroyed. If you only have one reference to an object stored in a
lexical scalar, the object is destroyed when that scalar goes out of
scope. If you store the object in a package global, that object may not
go out of scope until the program exits.

(the last sentence being the one that applies here) and

The order in which objects are destroyed during the global destruction
before the program exits is unpredictable.

I do not think it's a good idea to have destructors with
externally-visible effects happening in an undefined order. The present
bug is exactly because those things are happening in the "wrong" order.
Doubling down on using the DESTROY method won't make that better.

Now, whether using END is really an improvement is a separate question.
I have the impression that END calls happen in a better-defined order,
but I'm not a perl monk.

regards, tom lane