dbi icon indicating copy to clipboard operation
dbi copied to clipboard

Allow an inner handle to be hot-swapped from inside a callback.

Open robschaber opened this issue 8 years ago • 2 comments

Hello!

I'd like to be able to swap out an inner handle for a new one from inside a callback. This would allow me to test the connection and replace it if necessary before dispatch. Currently, it's not possible. This test script illustrates:

use DBI;
my $dbh = DBI->connect('dbi:ExampleP:');
printf "inner handle before dispatch: %s\n", tied %$dbh;
$dbh->{Callbacks} = { ping => \&swapper };
*DBD::ExampleP::db::ping = sub {
    my ($inner) = @_;
    printf "inner handle during dispatch: %s\n", $inner;
};
$dbh->ping;
printf "inner handle after dispatch:  %s\n", tied %$dbh;
exit;
sub swapper {
    my ($dbh) = @_;
    my $new_dbh = DBI->connect('dbi:ExampleP:');
    $dbh->swap_inner_handle($new_dbh);
    return;
}

On my machine this produces:

inner handle before dispatch: DBI::db=HASH(0x7ff2d383f8f0)
panic: attempt to copy freed scalar 7ff2d5080210 to 7ff2d5080960 at
    test.pl line 6.
perl(5734,0x7fff75766300) malloc: *** error for object 0x7ff2d3425940:
    incorrect checksum for freed object - object was probably modified
    after being freed.
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

The inner handle being used by the dispatcher was freed after my callback exited because it had been associated with the newly-created outer handle, which was garbage collected. I've patched the dispatcher to detect this situation and update its internal state accordingly (h, mg, imp_xxh). After applying the patch:

inner handle before dispatch: DBI::db=HASH(0x7fae998776f0)
inner handle during dispatch: DBI::db=HASH(0x7fae99877ff0)
inner handle after dispatch:  DBI::db=HASH(0x7fae99877ff0)

t/70callbacks.t has also been updated with a representative test case.

I've checked that no memory leaks have been introduced by running the test snippet inside a loop while watching memory usage. I also ran it in Xcode with all the memory-debugging features enabled, with no issues reported.

Thanks for taking a look, and let me know if I missed anything.

robschaber avatar Jun 23 '17 22:06 robschaber

Umm. That's quite a scary thing to be doing. I don't have any fundamental objection but do have some concerns...

  • duplication of code to set mg, h, and imp_xxh
  • but it doesn't use dbih_getcom2(aTHX_ h, 0); without explaining why not
  • no check for same thread (imp_xxh && DBIc_THR_USER(imp_xxh) != my_perl)
  • needs a debug log message
  • some docs would be good, even better if they say it's not supported :)

timbunce avatar Aug 06 '17 14:08 timbunce

Would not it better to introduce a $dbh->connect() method which just re-connect? When called without arguments it re-use arguments passed to DBI->connect() which returned $dbh.

@robschaber: I think that this solution is less ugly and better fit into DBI API as it already has $dbh->disconnect method.

pali avatar Jan 25 '19 14:01 pali