Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import 'import' instead of inheriting it #11

Merged
merged 2 commits into from
Jan 23, 2014
Merged

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