Skip to content

Loading…

DBI $dbh unconditionally disconnected by DBIx::Connector's DESTROY method #6

Closed
siracusa opened this Issue · 10 comments

2 participants

@siracusa

The documentation says the connect() class method can be used to get a DBI $dbh:

my $dbh = DBIx::Connector->connect($dsn, $username, $password, \%attr);

It goes on to say:

Though there's probably not much point in that, as you'll generally want to hold on to the DBIx::Connector object. Otherwise you'd just use the DBI, no?"

It's actually worse than that, since when the transiently created DBIx::Connector object goes out of scope and is DESTROYed, the DBI $dbh is disconnect()ed, rendering the $dbh returned pretty much useless.

I'm thinking of incorporating DBIx::Connector into an existing module that, among other things, expects to be able to create and then return a DBI $dbh using a transiently created object:

my $dbh = My::Whatever->new(...)->dbh;

If I use DBIx::Connector internally to create my $dbh, I'm faced with the same problem described above. Once the transiently created My::Whatever object goes out of scope, the DBIx::Connector object it used internally to create its $dbh also goes out of scope, and the $dbh is disconnected right before it's returned to the caller.

I could hack this by reaching into the DBIx::Connector object and undef-ing its "_dbh" attribute, but that's pretty evil. Since DBIx::Connector's connect() method already purports to do what I want, how about making a change that a) makes connect() work as advertised, and b) also provides a public API for telling a DBIx::Connector object not to disconnect() its $dbh in its DESTROY method?

@theory
Owner

Thanks, fixed a here. But I'm not sure that b) is needed. Do you have a use case for it?

@siracusa

Yes, my use case for b) is as I described. I have an existing module that has an API that allows the $dbh it contains to outlive the object that created it. In order for me to change my module to use DBIx::Connector internally, I'll need some way to make the $dbh outlive the (internally created) DBIx::Connector object as well. Note that the DBIx::Connector object may be created within my object, used many times internally, and only later asked to surrender its $dbh without disconnecting it even as the parent object goes out of scope. Example:

my $dbh;

if (...) {
    # $o creates a DBIx::Connector object internally
    my $o = My::Whatever->new(...);

    # $o uses its DBIx::Connector object to do work
    $o->do_something(...);
    ...

    # Retain an outside copy of $dbh
    $dbh = $o->retain_dbh;
}

# $o is now destroyed, and with it the DBIx::Connector object it 
# contained, but the $dbh that it returned earlier is still valid.
$dbh->do(...);
@theory
Owner

Okay, I'm convinced. Got a suggestion for a decent interface? I'm thinking an instance method just like dbh, like your retain_dbh example.

Funny. I've been doing iPad hacking lately, so thanks to Objective C, I know exactly why you use "retain" there. I'm not sure that suggests "Give me a database handle, but don't disconnect it when you go out of scope" to Perl hackers. Got another suggestion? I'd use "persistent", but that's not right, either.

@siracusa

I learned reference counting in Perl long before I knew Objective-C :) Anyway, in this case, I think all that's required is a way to tell the DBIx::Connector object not to call disconnect() during its DESTROY. So how about just a boolean object attribute method called disconnect_dbh_on_destroy([BOOL]) or something similar. It'll probably be rarely used, so the long name doesn't bother me, and it's pretty self-documenting.

@theory
Owner

How about just disconnect_on_destroy()?

@siracusa

That's actually what I had originally written, but I added the dbh because I figured why not err on the side of explicitness for a rarely used method like this. Either one is fine with me.

@theory
Owner

Add disconnect_on_destroy().

Suggested by John Siracusa. Closed by 582a1dd.

@theory
Owner

That work?

@siracusa

Looks great, thanks for the speedy reply!

@theory
Owner

0.41 on its way to a CPAN mirror near you.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.