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

Warnings from Test2::API during global destruction #827

Closed
atoomic opened this issue Mar 5, 2019 · 3 comments
Closed

Warnings from Test2::API during global destruction #827

atoomic opened this issue Mar 5, 2019 · 3 comments

Comments

@atoomic
Copy link
Contributor

atoomic commented Mar 5, 2019

As showed by the perl script just after when running it, this raises warnings from Test2::API during Global destruction.
I can reproduce this behavior using

  • Perl 5.26.0 which comes with Test-Simple 1.302073
  • Perl 5.28.0 with Test-Simple 1.302140

I tried to reduce it to a minimal script to trigger the warnings.
Note that removing a module, which can seem useless or duplicate would fix the issue, as I guess the global destruction will happen in a different order.

#!/usr/bin/env perl

use strict;
use warnings;

use Test2::Bundle::Extended;
use Test::MockModule;

package Test::Object;

use Test::More;

sub new {
    my ( $pkg, %opts ) = @_;

    my $self = bless { %opts }, $pkg;

    note "Create a new object: ", $self, undef;

    return $self;
}

sub DESTROY {
    my ( $self ) = @_;
    note "destroy time...", $self;

    return;
}

package main;

use Test2::Bundle::Extended;
use Test2::Tools::Explain;
use Test2::Plugin::NoWarnings;

my $obj = Test::Object->new;
ok $obj, 'new';

done_testing;

sub refcnt_plusplus {
    $obj //= 0;
}

output when running using Perl 5.28.0 with Test-Simple 1.302140

perl test.pl
# Seeded srand with seed '20190305' from local date.
# Create a new object: Test::Object=HASH(0x111e328)undef
ok 1 - new
1..1
Use of uninitialized value $hid in hash element at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API.pm line 327 during global destruction.
Use of uninitialized value $hid in hash element at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API.pm line 421 during global destruction.
Use of uninitialized value $hid in hash element at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API.pm line 422 during global destruction.
Use of uninitialized value $hid in hash element at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API/Context.pm line 129 during global destruction.
Use of uninitialized value $hid in hash element at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API/Context.pm line 129 during global destruction.
Use of uninitialized value $hid in delete at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API/Context.pm line 133 during global destruction.
	(in cleanup) 	(in cleanup)  at /usr/local/cpanel/3rdparty/perl/528/lib/perl5/cpanel_lib/Test2/API.pm line 285 during global destruction.

As shown above we are using $hid with an undefined value as a hash key... we should probably check if it's defined or not... but this might cover a larger bug.

@atoomic
Copy link
Contributor Author

atoomic commented Mar 5, 2019

of course adding undef $obj before done_testing provides a clean workaround and avoid the global destruction issue

@exodist
Copy link
Member

exodist commented Mar 5, 2019

done_testing signals the test internals that no more test events will happen and it is safe to output the final test count and other results, and safe for destruction to happen. Your note in the DESTROY block is pretty much crafted to send a test event after done_testing and after the test framework has started letting references get garbage collected.

There really is no way to protect against this, once Test::Builder/Test2 have started garbage collecting it is not safe to modify the state, which may not be around anymore to be modified.

It might be possible to add a better message when it detects this has happened, something like "Attempt to send a test event after test state teardown", but that is still not super clear. Also it would be almost impossible to catch every place this kind of thing can happen.

@exodist
Copy link
Member

exodist commented Apr 25, 2019

I just merged a change that makes the error message a lot more useful:

# Seeded srand with seed '20190425' from local date.
# Create a new object: Test::Object=HASH(0x55898c2a4578)undef
ok 1 - new
1..1
Attempt to get Test2 context after testing has completed (did you attempt a testing event after done_testing?) at /home/exodist/projects/Test2/Test-More/lib/Test2/API.pm line 269.
        Test2::API::context_do(CODE(0x55898cc077c8), "Use of uninitialized value \$hid in hash element at /home/exod"...) called at /home/exodist/perl5/perlbrew/perls/main/lib/site_perl/5.28.1/Test2/Plugin/NoWarnings.pm line 33
        Test2::Plugin::NoWarnings::__ANON__("Use of uninitialized value \$hid in hash element at /home/exod"...) called at /home/exodist/projects/Test2/Test-More/lib/Test2/API.pm line 343
        Test2::API::context("level", 2, "fudge", 1, "stack", Test2::API::Stack=ARRAY(0x55898c675018), "hub", undef, ...) called at /home/exodist/projects/Test2/Test-More/lib/Test/Builder.pm line 152
        Test::Builder::ctx(Test::Builder=HASH(0x55898c2cb348)) called at /home/exodist/projects/Test2/Test-More/lib/Test/Builder.pm line 404
        Test::Builder::reset(Test::Builder=HASH(0x55898c2cb348), "singleton", 1) called at /home/exodist/projects/Test2/Test-More/lib/Test/Builder.pm line 105
        Test::Builder::__ANON__() called at /home/exodist/projects/Test2/Test-More/lib/Test2/API/Instance.pm line 283
        Test2::API::Instance::add_post_load_callback(Test2::API::Instance=HASH(0x55898c52b6a8), CODE(0x55898cd6ffb0)) called at /home/exodist/projects/Test2/Test-More/lib/Test2/API.pm line 205
        Test2::API::test2_add_callback_post_load(CODE(0x55898cd6ffb0)) called at /home/exodist/projects/Test2/Test-More/lib/Test/Builder.pm line 108
        Test::Builder::new("Test::Builder") called at /home/exodist/projects/Test2/Test-More/lib/Test/Builder/Module.pm line 172
        Test::Builder::Module::builder("Test::More") called at /home/exodist/projects/Test2/Test-More/lib/Test/More.pm line 1259
        Test::More::note("destroy time...", Test::Object=HASH(0x55898c2a4578)) called at xxx.t line 23
        Test::Object::DESTROY(Test::Object=HASH(0x55898c2a4578)) called at xxx.t line 0
        eval {...} called at xxx.t line 0
        (in cleanup)    (in cleanup)  at /home/exodist/projects/Test2/Test-More/lib/Test2/API.pm line 339 during global destruction.

exodist added a commit that referenced this issue Apr 25, 2019
    - Do not use threads::Shared in Test::Tester::Capture (#826)
    - Add missing version info to Info/Table
    - Fix event in global destruction bug (#827)
    - Proper fix for todo = '' (#812, #829)
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

No branches or pull requests

2 participants