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
Dave Mitchell
June 15, 2018 12:52
Re: [perl #133270] Blead Breaks CPAN: HMBRAND/DBD-CSV-0.53.tgz
Message ID:
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:


    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

    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
    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));


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

    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

        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
                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 Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About