From dcb2da03245977d3524545740fce3750e046b1fc Mon Sep 17 00:00:00 2001 From: Rob Schaber Date: Fri, 23 Jun 2017 16:52:19 -0500 Subject: [PATCH 1/3] Allow an inner handle to be hot-swapped from inside a callback. --- DBI.xs | 27 ++++++++++++++++++++++++++- t/70callbacks.t | 22 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/DBI.xs b/DBI.xs index ae44d97e..e3230736 100644 --- a/DBI.xs +++ b/DBI.xs @@ -3594,6 +3594,13 @@ XS(XS_DBI_dispatch) # endif DEFSV_set(temp_defsv); + /* We probably localized a member of imp_xxh earlier (call_depth) so + * we need to keep a reference to its inner handle owner until the + * end of scope so that it can be restored if our callback swaps out + * the handle and tries to garbage collect it. + */ + sv_2mortal(newRV_inc(h)); + EXTEND(SP, items+1); PUSHMARK(SP); PUSHs(orig_h); /* push outer handle, then others params */ @@ -3604,6 +3611,24 @@ XS(XS_DBI_dispatch) outitems = call_sv(code, G_ARRAY); /* call the callback code */ MSPAGAIN; + /* Callback may have swapped out the inner handle from underneath us. + * If so, we need to refresh our vars (h, mg, imp_xxh) before dispatch. + */ + { + MAGIC *outer_magic = SvMAGIC(SvRV(orig_h)); + if (outer_magic->mg_type != 'P') + outer_magic = mg_find(SvRV(h), 'P'); + if (outer_magic != mg) { + SV *inner_handle = outer_magic->mg_obj; + MAGIC *inner_magic = SvMAGIC(SvRV(inner_handle)); + if (inner_magic->mg_type != DBI_MAGIC) + inner_magic = mg_find(SvRV(inner_handle), DBI_MAGIC); + mg = outer_magic; + h = ST(0) = inner_handle; + imp_xxh = (imp_xxh_t *)inner_magic->mg_ptr; + } + } + /* The callback code can undef $_ to indicate to skip dispatch */ skip_dispatch = !SvOK(DEFSV); /* put $_ back now, but with an incremented ref count to compensate @@ -5135,7 +5160,7 @@ take_imp_data(h) PUSHMARK(sp); XPUSHs(*hp); PUTBACK; - call_method("finish", G_VOID); + call_method("finish", G_VOID|G_DISCARD); SPAGAIN; PUTBACK; sv_unmagic(SvRV(*hp), 'P'); /* untie */ diff --git a/t/70callbacks.t b/t/70callbacks.t index 40903f98..f64ae03c 100644 --- a/t/70callbacks.t +++ b/t/70callbacks.t @@ -232,6 +232,28 @@ my $stress_sth = $stress_dbh->prepare("select 1"); $stress_sth->{Callbacks}{execute} = sub { return; }; $stress_sth->execute(@params); +# Test that a db handle can be swapped out for a newly-created +# one from inside a callback with no ill effects. +{ + my $dbh = DBI->connect($dsn); + my $old = tied %$dbh; + my $cb = sub { + my ($dbh) = @_; + my $new_dbh = DBI->connect($dsn); + $dbh->swap_inner_handle($new_dbh); + return; + }; + $dbh->{Callbacks} = { ping => $cb }; + no warnings qw/once redefine/; + local *DBD::ExampleP::db::ping = sub { + my ($dbh) = @_; + my $new = tied %$dbh; + isnt $old, $new, 'inner handle replaced inside method'; + }; + $dbh->ping(); + my $new = tied %$dbh; + isnt $old, $new, 'inner handle replaced after method'; +} done_testing(); From a06b7852c28f26bc2f427838937f346b6e4da1a2 Mon Sep 17 00:00:00 2001 From: Rob Schaber Date: Fri, 23 Jun 2017 18:06:33 -0500 Subject: [PATCH 2/3] Fix improper test comparison in t/70callbacks.t A dispatched method receives the inner handle, not outer. --- t/70callbacks.t | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/70callbacks.t b/t/70callbacks.t index f64ae03c..d67622af 100644 --- a/t/70callbacks.t +++ b/t/70callbacks.t @@ -246,8 +246,7 @@ $stress_sth->execute(@params); $dbh->{Callbacks} = { ping => $cb }; no warnings qw/once redefine/; local *DBD::ExampleP::db::ping = sub { - my ($dbh) = @_; - my $new = tied %$dbh; + my ($new) = @_; isnt $old, $new, 'inner handle replaced inside method'; }; $dbh->ping(); From c14b45aecb6daef4720d292faacdf203d2ef5279 Mon Sep 17 00:00:00 2001 From: Rob Schaber Date: Fri, 23 Jun 2017 18:34:57 -0500 Subject: [PATCH 3/3] Get outer handle magic from the outer handle, not inner handle During development, forgot to fix the secondary lookup after fixing the primary one. --- DBI.xs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DBI.xs b/DBI.xs index e3230736..be2e0c09 100644 --- a/DBI.xs +++ b/DBI.xs @@ -3617,7 +3617,7 @@ XS(XS_DBI_dispatch) { MAGIC *outer_magic = SvMAGIC(SvRV(orig_h)); if (outer_magic->mg_type != 'P') - outer_magic = mg_find(SvRV(h), 'P'); + outer_magic = mg_find(SvRV(orig_h), 'P'); if (outer_magic != mg) { SV *inner_handle = outer_magic->mg_obj; MAGIC *inner_magic = SvMAGIC(SvRV(inner_handle));