Skip to content

Import 'import' instead of inheriting it#11

Merged
doy merged 2 commits intodoy:masterfrom
ilmari:global-destruction
Jan 23, 2014
Merged

Import 'import' instead of inheriting it#11
doy merged 2 commits intodoy:masterfrom
ilmari:global-destruction

Conversation

@ilmari
Copy link

@ilmari ilmari commented Oct 21, 2013

This fixes errors when the first use of Try::Tiny happens during global
destruction on Perl >= 5.10.0.

The problem is that the inheritance caches don't get invalidated during
global destruction, and base.pm skips classes that the calling class
already inherits from, thus populating an initial empty inheritance
cache, which doesn't get invalidated when it adds the class to @isa.

This fixes errors when the first use of Try::Tiny happens during global
destruction on Perl >= 5.10.0.

The problem is that the inheritance caches don't get invalidated during
global destruction, and base.pm skips classes that the calling class
already inherits from, thus populating an initial empty inheritance
cache, which doesn't get invalidated when it adds the class to @isa.
@doy
Copy link
Owner

doy commented Oct 21, 2013

Do you have a test for this? I can't seem to reproduce it (I get this same result on all perls from 5.8 to 5.18):

$ perl -e'{ package Foo; sub DESTROY { require Try::Tiny; Try::Tiny::try { warn "foo" } } }; our %foo; bless \%foo, "Foo";' 
foo at -e line 1 during global destruction.

@ilmari
Copy link
Author

ilmari commented Oct 21, 2013

The problem is caused when a module importing try/catch from Try::Tiny is loaded during global destruction, in which case the subs aren't exported to it, since 'use base' doesn't work.

Because exceptions during global destruction are converted to warnings and don't affect the exit status, testing would involve using Capture::Tiny, which until yesterday (version 0.23) didn't install on 5.6. Now that that's fixed, I'll gladly write some tests, if that's an acceptable test prereq.

@doy
Copy link
Owner

doy commented Oct 21, 2013

Yes, that's fine - just make it an author dependency (and have the test skip if it's not installed).

doy added a commit that referenced this pull request Jan 23, 2014
Import 'import' instead of inheriting it
@doy doy merged commit ded671b into doy:master Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants