Bug #111181 for Test-postgresql: Cleanup not working because of DESTROY order

Wed Jan 13 21:18:45 2016 sargie [...] cpan.org - Ticket created

Subject:

Cleanup not working because of DESTROY order

I don't think this is the same bug as https://rt.cpan.org/Ticket/Display.html?id=108460 - apologies if I've got that wrong. base_dir is a File::Temp::Dir, however if Test::PostgreSQL is killed due to an error, DESTROY gets called, which calls ->stop(), which relies on base_dir still being around to create a sensible pg_ctrl statement. pg_ctl: directory "/data" does not exist (in cleanup) /usr/local/bin/pg_ctl stop -s -D /data -m fast failed:256 In my case I have a singleton, so I've just put an explicit stop in an END{} block, which works.

Wed Jan 13 21:56:31 2016 TJC [...] cpan.org - Taken

Wed Jan 13 22:00:30 2016 TJC [...] cpan.org - Correspondence added

Hi, I wondered if the current dev version on CPAN, 1.20_03, behaves any differently in this regard? It's a Moo conversion, so the stop method is now called from inside a DEMOLISH handler rather than DESTROY.

Wed Jan 13 22:00:30 2016 The RT System itself - Status changed from 'new' to 'open'

Thu Jan 14 09:14:44 2016 SYSPETE [...] cpan.org - Correspondence added

On Wed Jan 13 21:18:45 2016, SARGIE wrote: Show quoted text

> I don't think this is the same bug as > https://rt.cpan.org/Ticket/Display.html?id=108460 - apologies if I've > got that wrong.

This is definitely a new bug - thanks for submitting. Show quoted text

> base_dir is a File::Temp::Dir, however if Test::PostgreSQL is killed > due to an error, DESTROY gets called, which calls ->stop(), which > relies on base_dir still being around to create a sensible pg_ctrl > statement. > > pg_ctl: directory "/data" does not exist > (in cleanup) /usr/local/bin/pg_ctl stop -s -D /data -m fast > failed:256 > > In my case I have a singleton, so I've just put an explicit stop in an > END{} block, which works.

Do you have a test script or a repeatable scenario you could share that causes this as I'm having trouble recreating this problem locally? Cheers PeteM

Thu Jan 14 18:41:16 2016 sargie [...] cpan.org - Correspondence added

On Thu Jan 14 09:14:44 2016, SYSPETE wrote: Show quoted text

> Do you have a test script or a repeatable scenario you could share > that causes this as I'm having trouble recreating this problem > locally?

My understanding of how global DESTROY works - such that its ordering is unpredictable - means that it's not possible to create a repeatable scenario for this.

Thu Jan 14 20:04:20 2016 TJC [...] cpan.org - Correspondence added

Just a thought, but what if we created a subclass of File::Temp and then overrode its DESTROY method (and constructor) so that it can take a callback to run in DESTROY? We then use that to call ->stop before cleanup. Back in Test Postgresql, we make stop() happy to be called multiple times safely, as our own destroy handler might get called first some times. That proposal doesn't make me happy, but it came to mind as a potential fix.

Thu Jan 14 20:06:30 2016 sargie [...] cpan.org - Correspondence added

On Thu Jan 14 20:04:20 2016, TJC wrote: Show quoted text

> That proposal doesn't make me happy, but it came to mind as a > potential fix.

That could work, but I agree with your assessment of 'eww'. What about adding an END block that calls DESTROY?

Thu Jan 14 20:12:02 2016 TJC [...] cpan.org - Correspondence added

On Thu Jan 14 20:06:30 2016, SARGIE wrote: Show quoted text

> That could work, but I agree with your assessment of 'eww'. What about > adding an END block that calls DESTROY?

Does an END block get called with the object reference ($self) in the same way that DESTROY does?

Fri Jan 15 06:13:24 2016 SYSPETE [...] cpan.org - Correspondence added

On Thu Jan 14 20:12:02 2016, TJC wrote: Show quoted text

> On Thu Jan 14 20:06:30 2016, SARGIE wrote:

> > That could work, but I agree with your assessment of 'eww'. What > > about > > adding an END block that calls DESTROY?

> > Does an END block get called with the object reference ($self) in the > same way that DESTROY does?

The solution I think is even simpler: if base_dir is undef due to its File::Temp obj having been destroyed then just shutdown Pg using a signal instead. We already have this code for Pg<v9 so it makes a good escape route for this situation: https://github.com/SysPete/Test-postgresql/commit/0853f57683a42ed59e8c82ae79ca0f2aba64d999 The commit message hopefully summarises the issue and solution clearly.

Fri Jan 15 06:20:31 2016 SYSPETE [...] cpan.org - Correspondence added

On Fri Jan 15 06:13:24 2016, SYSPETE wrote: Show quoted text

> On Thu Jan 14 20:12:02 2016, TJC wrote:

> > On Thu Jan 14 20:06:30 2016, SARGIE wrote:

> > > That could work, but I agree with your assessment of 'eww'. What > > > about > > > adding an END block that calls DESTROY?

> > > > Does an END block get called with the object reference ($self) in the > > same way that DESTROY does?

> > The solution I think is even simpler: if base_dir is undef due to its > File::Temp obj having been destroyed then just shutdown Pg using a > signal instead. We already have this code for Pg<v9 so it makes a good > escape route for this situation: > > https://github.com/SysPete/Test- > postgresql/commit/0853f57683a42ed59e8c82ae79ca0f2aba64d999 > > The commit message hopefully summarises the issue and solution > clearly.

Updated commit (added ticket number + attribution to SARGIE): https://github.com/SysPete/Test-postgresql/commit/03b3b6b00ec33dabe8fa44269ffed573c23d7719

Mon Jan 18 22:11:20 2016 TJC [...] cpan.org - Correspondence added

Thanks for the pull request. I've merged it and released 1.20_04 containing that patch. Resolving ticket.

Mon Jan 18 22:11:21 2016 TJC [...] cpan.org - Status changed from 'open' to 'resolved'