Re^6: method chaining fails where separate method calls succeed in DBIx::Simple (Moose)

[tye]

So, if you take Moose out of the picture, does the problem go away? That would be the first place I would suggest slicing the problem to narrow down the source (of the real bug that is exposed by the quoting of $self).

Though, I suspect trying that will point the finger at Moose not away from it. And finding a bug in the huge volumes of code that Moose pulls in could be a daunting task. But it doesn't look like your test case uses a lot of Moose features, so, if Moose appears required to reproduce the problem, then you can probably start cutting out bits on that side to narrow things down much smaller than just "something Moose requires".

For example, try replacing Moose with Mouse and see if the problem still exists.

[metaperl]

For example, try replacing Moose with Mouse and see if the problem still exists.

Mouse does build on Strawberry Perl 5.10. Moo does, but default values must be a sub ref so I rewrote that part of the test. Also apparently Mouse does use some XS, whereas Moo does not.

And with Moo, the problem still exists. Here is the new code.

[tye]

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

Read more... (4 kB)

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:

Read more... (4 kB)

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

        }

[Juerd]

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.

[metaperl]

Excellent work, tye. I've cc'ed you on the RT Ticket I just filed.

Personally DBIx::Custom looks to give me all that DBIx::Simple has supplied me for all these years in a new more modern package with the benefit of hindsight.

Instead of forking Simple, I think I will just move over to Custom.