in reply to Re^7: method chaining fails where separate method calls succeed in DBIx::Simple (Moose)
in thread method chaining fails where separate method calls succeed in DBIx::Simple

Having a runnable test case that didn't require spending hours trying to get stuff installed helped clarify the described problem.

The basic problem is that the DBIx::Simple object [returned from dbs()] gets destroyed after the DBIx::Simple::Statement object is created [and returned by query()] but before it can be used.

It wasn't, as I initially misunderstood, that the DBIx::Simple::Statement object is actually being DESTROYed before it can be used.

DBIx::Simple goes to some significant lengths to make all DBIx::Simple::Statement objects suddenly become unusable as soon as their parent DBIx::Simple object is destroyed.

I don't pretend to know why this strange lifecycle interplay is implemented or even whether or not it is a good idea.

But thwarting that part of the module design by inducing circular references such that things just never get destroyed is not what I would call a "bug fix", nor "wise".

Here is an abbreviated summary of the differences between runs of your test cases with one I added with and without the "fix" of not quoting $self (matching lines are prefixed with "=" to aid comparison): 

=Starting CASE 1
=...
    'db' => 'DBIx::Simple=...',
=...
=Hashes? $VAR1 = [];
=ok 1 - object isa DBIx::Simple::Statement
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::DeadObject
=Destroying DBIx::Simple::DeadObject
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::DeadObject
=Starting CASE 2
=...
    'db' => 'DBIx::Simple=...',
=...
=Hashes? $VAR1 = [];
=ok 2 - object isa DBIx::Simple::Statement
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::DeadObject
=Destroying DBIx::Simple::DeadObject
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::DeadObject
=Starting CASE 3
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
not ok 3 - object isa ...
#   Failed test ...
=Destroying DBIx::Simple::Result 1
(only one Result got created?)
=Destroying DBIx::Simple::DeadObject 1
(only one DeadObject got created?)
=Starting CASE 4
=...
    'db' => 'DBIx::Simple=...',
=...
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::Result
Destroying DBIx::Simple::DeadObject
Result object no longer usable
(no output)
(not even failed test report)
(destroyed above)
(only one Result created?)
(destroyed above)
=Destroying DBIx::Simple::DeadObject 2
=# Tests were run but no plan ...
=(global destruction begins)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)
(destroyed above)

[download]
=Starting CASE 1
=...
    'db' => bless( { ...
=...
=Hashes? $VAR1 = [];
=ok 1 - object isa DBIx::Simple::Statement
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::DeadObject
=Destroying DBIx::Simple::DeadObject
(not destroyed yet)
(not destroyed yet)
(not destroyed yet)
=Starting CASE 2
=...
    'db' => bless( { ...
=...
=Hashes? $VAR1 = [];
=ok 2 - object isa DBIx::Simple::Statement
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::Result
=Destroying DBIx::Simple::DeadObject
=Destroying DBIx::Simple::DeadObject
(not destroyed yet)
(not destroyed yet)
(not destroyed yet)
=Starting CASE 3
(not destroyed yet)
(not destroyed yet)
ok 3 - object isa DBIx::Simple::Statement
(doesn't fail)
=Destroying DBIx::Simple::Result 1
Destroying DBIx::Simple::Result 2
=Destroying DBIx::Simple::DeadObject 1
Destroying DBIx::Simple::DeadObject 2
=Starting CASE 4
=...
    'db' => bless( { ...
=...
(not destroyed yet)
(not destroyed yet)
(not destroyed yet)
(not destroyed yet)
(object still usable)
Hashes? $VAR1 = [];
ok 4 - object isa DBIx::Simple::Statement
Destroying DBIx::Simple::Result
Destroying DBIx::Simple::Result
Destroying DBIx::Simple::DeadObject
=Destroying DBIx::Simple::DeadObject 2
=# Tests were run but no plan ...
=(global destruction begins)
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::DeadObject
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::DeadObject
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple
Destroying DBIx::Simple::DeadObject
Destroying DBIx::Simple
Destroying DBIx::Simple::Statement
Destroying DBIx::Simple::DeadObject

[download]

Which shows that the 'fix' does indeed prevent a bunch of stuff from being cleaned up until Perl's "global destruction".

Here is my modified test code. The modifications I made to DBIx::Simple are left as a trivial exercise for the reader: 

# use Devel::SimpleTrace;

{

    package Util;

    sub dbconnect {
        use DBI;
        DBI->connect('dbi:SQLite:temp.db');
    }

}

{

    package Local::DBIx::Simple::Q;

    use Moo;

    has 'q' => ( is => 'rw', default => sub { $main::backgroundqueue }
+ );
    has 'standard' => ( is => 'rw', default => sub { 0 } );

    use Data::Dumper;

    sub BUILD {
        my ($self) = @_;
        $main::globalstandardconnection = $self->standard

    }

    sub enq {
        my ( $self, @arg ) = @_;
        warn sprintf "Enqueing with id %d this data: %s", $self->enq_i
+d,
          Dumper( \@arg );
        $self->q->enqueue( [ $self->enq_id, @arg ] );
    }

}

{

    package Local::DBIx::Simple;

    use Moo;
    extends qw(Local::DBIx::Simple::Q);

    use DBIx::Simple;

    has 'enq_id' => ( is => 'rw', default => sub { 5 } );
    has 'deq_id' => ( is => 'rw', default => sub { 6 } );

    sub dbh {

        Util::dbconnect;
    }

    sub dbs {
        my ($self) = @_;

        my $dbs = DBIx::Simple->connect( $self->dbh );
    }

}

{

    package main;

    use strict;
    use warnings;

    use Data::Dumper;
    use Test::More;

    use lib 'lib';

    sub constructor {
        Local::DBIx::Simple->new( standard => 0 );
    }

    sub create_database {
        my ($dbh) = @_;

        my $ddl = <<'EODDL';
create table table_one (
  col1 integer not null primary key,
  col2 TEXT
)
EODDL

        $dbh->do($ddl);
    }

    sub main {

        my $dbh = Util::dbconnect;
        create_database($dbh);

        my $Q = "SELECT * FROM table_one";

        my $desired_class = 'DBIx::Simple::Statement';
        my $desired_desc  = "object isa $desired_class";

        warn "Starting CASE 1";
        {    # CASE 1 - successful
            my $s   = constructor;
            my $dbs = DBIx::Simple->connect( $s->dbh );
            my $r   = $dbs->query($Q);

            warn sprintf 'Result of DBIx::Simple query: %s', Dumper($r
+);
            my $h = $r->hashes;
            warn sprintf 'Hashes? %s', Dumper($h);
            ok( $r->{st}->isa($desired_class), $desired_desc );
        }

        warn "Starting CASE 2";
        {    # CASE 2 - successful
            my $s   = constructor;
            my $dbs = $s->dbs;
            my $r   = $dbs->query($Q);
            warn sprintf 'Result of DBIx::Simple-from-Local query: %s'
+,
              Dumper($r);
            my $h = $r->hashes;
            warn sprintf 'Hashes? %s', Dumper($h);
            ok( $r->{st}->isa($desired_class), $desired_desc );

        }

        warn "Starting CASE 3";
        {    # CASE 3 - *FAILS* when $self is quoted on line 165
            my $s = constructor;
            my $r = $s->dbs->query($Q);
            ok( $r->{st}->isa($desired_class), $desired_desc );

        }

        warn "Starting CASE 4";
        {    # CASE 4 - also fails
            my $s   = constructor;
            my $r;
            {
                my $dbs = $s->dbs;
                $r   = $dbs->query($Q);
                warn sprintf 'Result of DBIx::Simple-from-Local query:
+ %s',
                  Dumper($r);
            }
            my $h = $r->hashes;
            warn sprintf 'Hashes? %s', Dumper($h);
            ok( $r->{st}->isa($desired_class), $desired_desc );

        }
    }
}

main() unless caller;

1;

[download]

The test case I added makes it clearer how the lifecycle interplay designed into the module is violated: 

        warn "Starting CASE 4";
        {    # CASE 4 - also fails
            my $s   = constructor;
            my $r;
            {
                my $dbs = $s->dbs;
                $r   = $dbs->query($Q);
                warn sprintf 'Result of DBIx::Simple-from-Local query:
+ %s',
                  Dumper($r);
            }
            my $h = $r->hashes;
            warn sprintf 'Hashes? %s', Dumper($h);
            ok( $r->{st}->isa($desired_class), $desired_desc );

        }

[download]

- tye        

  • Comment on Re^8: method chaining fails where separate method calls succeed in DBIx::Simple (lifecycle)
  • Select or Download Code

Replies are listed 'Best First'.
Re^9: method chaining fails where separate method calls succeed in DBIx::Simple (lifecycle)
by Juerd (Abbot) on Aug 31, 2011 at 14:26 UTC

    DBIx::Simple goes to some significant lengths to make all DBIx::Simple::Statement objects suddenly become unusable as soon as their parent DBIx::Simple object is destroyed.

    I don't pretend to know why this strange lifecycle interplay is implemented or even whether or not it is a good idea.

    DBIx::Simple objects represent database connections, and when your connection to the database is gone, the corresponding statement handles will no longer function correctly. Because destruction/disconnection related bugs can be hard to find, DBIx::Simple actively shuts down the remaining active statements, and replaces them with objects that when used throw an error message that actually contains information about where the object was destroyed.

    It is possible to build a DBIx::Simple object from an existing DBI connection, in which case destruction of the DBIx::Simple object does not cause disconnection. Whether statements should be kept around is debatable but I chose to keep it simple, and let DBIx::Simple clean its own mess regardless of how the database connection was originally made.

    The option to pass an existing $dbh was added later and it appears that a part of the documentation was not updated accordingly:

    Destroys (finishes) active statements and disconnects. Whenever the database object is destroyed, this happens automatically if DBIx::Simple handled the connection (i.e. you didn't use an existing DBI handle). After disconnecting, you can no longer use the database object or any of its result objects.
    Destruction used to unconditionally also disconnect the $dbh; this was changed later, but that made that last sentence incomplete. It should instead read "After disconnecting or destroying the DBIx::Simple object, ..."

    Although the documentation should be improved, DBIx::Simple is doing exactly what it was designed to do. Indeed, simply removing the quotes and making a real reference does not fix a bug, and it is certainly not a wise thing to do: it introduces new bugs, because users of DBIx::Simple depend on their objects being destroyed and their database connections disconnected when their $db goes out of scope.

    The trick for wrappers like metaperl's Local::DBIx::Simple could be to somehow keep a reference around and do some of their own lifecycle management.

[reply]

      Thanks for the explanation. That is about what I expected, actually even better.

      The trick for wrappers like metaperl's Local::DBIx::Simple could be to somehow keep a reference around and do some of their own lifecycle management.

      Exactly.

      - tye        

[reply]
      Because destruction/disconnection related bugs can be hard to find

      What is your opinion of DBIx::Connector? Do you think you should delegate the complexity of keeping database connections alive to it?

      Also, please note well that destruction bugs and disconnection bugs are two separate classes of problem. I dont know which the use of double quotes about $db was supposed to address. But I do know that common sense about reference counting between a DBIx::Simple database handle and DBIx::Simple::Statement which has-a database handle should not require any particular weakening like you are doing. Just think for a second:

      1. we create a database connection, D
      2. we create a statement instance, S, which refers to D. This makes the reference count for D== 2
      3. we create a another statement instance, S2, which also refers to D. This makes the reference count for D== 3
      4. S2 goes out of scope. Reference count for D drops to 2
      5. S goes out of scope. Reference count for D drops to 1
      6. D goes out of scope, reference count for D drops to 0 and D is extinguished
      I simply dont understand why we need to prevent the reference count from naturally increasing and decreasing as the DBIx::Simple instance becomes a compononent of DBIx::Simple::Statement instances.

      The trick for wrappers like metaperl's Local::DBIx::Simple could be to somehow keep a reference around and do some of their own lifecycle management.

      Without any concrete suggestions for modification, I dont know what to say or try. But I would say this. Local::DBIx::Simple is a very simple, clearly written wrapper and it isnt working.

[reply]
[d/l]

        What is your opinion of DBIx::Connector? Do you think you should delegate the complexity of keeping database connections alive to it?

        It solves a different, unrelated problem. I'm not inclined to let DBIx::Simple depend on anything but DBI, except optionally.

        Also, please note well that destruction bugs and disconnection bugs are two separate classes of problem. I dont know which the use of double quotes about $db was supposed to address.

        I'm aware. Avoiding circular references has to do with avoiding problems regarding destruction; in particular, avoiding that destruction never happens during runtime. Because in DBIx::Simple, destruction causes disconnection (iff you let DBIx::Simple create the connection), I mentioned them together.

        But I do know that common sense about reference counting between a DBIx::Simple database handle and DBIx::Simple::Statement which has-a database handle should not require any particular weakening like you are doing.

        Common sense dictates that one does not use a circular reference if one can easily do without. There's no reason for ::Statement to have-a database handle, so it doesn't has-a one at all. All it needs is to refer to the object, as a hash key. That's a string. So all we need is that string. Et voila, we've avoided circular references without the complexity and risks of weakening. You're referring to the trick I'm using as "weakening" but it is very different.

        I simply dont understand why we need to prevent the reference count from naturally increasing and decreasing as the DBIx::Simple instance becomes a compononent of DBIx::Simple::Statement instances.

        tye has explained this thoroughly, with examples of what happens. I'm sorry, but if you don't understand his explanation, you don't understand destruction and you should really never use, or suggest that anyone use, a circular reference.

        But I would say this. Local::DBIx::Simple is a very simple, clearly written wrapper and it isnt working.

        It is working. I believe that the problem you encounter lies somewhere within the libraries you're using for object management, or, possibly, perl itself.

        In any case, the workaround is extremely simple and clear and you've already discovered it yourself: use a variable.

[reply]
Re^9: method chaining fails where separate method calls succeed in DBIx::Simple (lifecycle)
by metaperl (Curate) on Aug 17, 2011 at 15:48 UTC
[reply]

In Section Seekers of Perl Wisdom