develooper Front page | perl.perl5.porters | Postings from June 2018

Re: [perl #133270] Blead Breaks CPAN: HMBRAND/DBD-CSV-0.53.tgz

Thread Previous | Thread Next
From:
Dave Mitchell
Date:
June 15, 2018 12:52
Subject:
Re: [perl #133270] Blead Breaks CPAN: HMBRAND/DBD-CSV-0.53.tgz
Message ID:
20180615125200.GB2753@iabyn.com
On Sun, Jun 10, 2018 at 09:36:22PM +0100, Dave Mitchell wrote:
> On Sat, Jun 09, 2018 at 11:05:23PM +0100, Dave Mitchell wrote:
> > I've now seen a failure under 5.26.0 (one failure in about 200 runs of
> > t/82_free_unref_scalar.t).
> 
> I've now bisected it (kinda). From at least 5.26.0 until the commit below,
> t/82_free_unref_scalar.t would typically trigger a failure once every 100
> runs or so, and that failure would typically involve a single double-freed
> SV. From the commit through to bleed, around 1 in 4 test runs fails,
> typically with several double-freed SVs.
> 
> I don't know yet whether blead has introduced or new bug, or whether it
> just makes the test more sensitive to an existing bug - a bug which could
> be in perl or one or more CPAN modules. :-(

I've now diagnosed the issues. It turns out that its not a perl issue.
but a number of bugs in various DBI / DBD components.

I've raised the following tickets:

---------------------------------------------------------------

https://rt.cpan.org/Ticket/Display.html?id=125589

    This code in hook() in CSV_XS.xs is problematic in two ways:

        XPUSHs (newRV_noinc ((SV *)hv));
        XPUSHs (newRV_noinc ((SV *)av));

    First, the newly-created RVs will leak, since they're not mortalised,
    and perl's arg stack isn't reference counted.

    Secondly, because the reference count of hv and av aren't increased,
    they may be prematurely freed and perl may subsequently crash.
    In fact, this is happening in

        https://rt.perl.org/Ticket/Display.html?id=133270

    In a stripped-down t/82_free_unref_scalar.t from DBD::CSV, I see

    1) the RV created by newRV_noinc ((SV *)av) lasting until global
    destruction;
    2) in the meantime, the AV it points to is (prematurely) freed, and the
       SV later happens to be reallocated as an HV which happens to survive
       until global destruction;
    3) When the RV is eventually freed during global destruction, it triggers
       a double free of that new HV.

    I suspect you instead want something like

        mXPUSHs (newRV_inc ((SV *)hv));
        mXPUSHs (newRV_inc ((SV *)av));

---------------------------------------------------------------

https://rt.cpan.org/Ticket/Display.html?id=125590

    While debugging an issue with DBD::CSV and/or perl:

        https://rt.perl.org/Ticket/Display.html?id=133270

    I spotted code in DBI::DBD::SqlEngine::TieTables which appears to
    be creating a circular reference, and thus leaking.

    DBI::DBD::SqlEngine::TieTables::TIEHASH() creates a tie object hash,
    one of whose entries is a reference back to the $dbh handle.

    I don't understand this code well enough to be 100% certain its a leak,
    but it sure looks like a weaken() is in order.

    The following code definitely leaks, although that may be in part due to
    a bug in Text::CSV_XS which I have reported separately:

    (it requires editing 'f_dir' to point to an untarred DBD-CSV-0.53
    directory)

        use strict;
        use warnings;

        use DBI;

        while (1) {
            my $dbh = DBI->connect ("dbi:CSV:", undef, undef, {
                f_schema         => undef,
                f_dir            => '/home/davem/tmp-nobackup/dbd-csv-free/DBD-CSV-0
    .53/t',
                f_dir_search     => [],
                f_ext            => ".csv/r",
                f_lock           => 2,
                f_encoding       => "utf8",

                csv_auto_diag => 0,

                RaiseError       => 1,
                PrintError       => 1,
                FetchHashKeyName => "NAME_lc",
                }) or die "$DBI::errstr\n" || $DBI::errstr;
        }

---------------------------------------------------------------



-- 
Any [programming] language that doesn't occasionally surprise the
novice will pay for it by continually surprising the expert.
   -- Larry Wall

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About