Better handling of magic methods freeing the SV
authorDavid Mitchell <davem@iabyn.com>
Thu, 30 Dec 2010 10:32:44 +0000 (10:32 +0000)
committerDavid Mitchell <davem@iabyn.com>
Thu, 30 Dec 2010 10:51:45 +0000 (10:51 +0000)
commit8985fe98dcc5c0af2fadeac15dfbc13f553ee7fc
treec46242d08c7fce5cf802e91d2b5ed245074c049c
parent117a8c22e551541bfbe9b2b8169cd68d3321217a
Better handling of magic methods freeing the SV

This is a fix for RT #81230 (and more). Currently, mg_get() works around
the case where the called magic (e.g. FETCH) frees the magic SV. It does
this by unconditionally pushing the SV on the tmps stack before invoking
the method.

There are two issues with this. Firstly, it may artificially extend the
life of the SV. This was the root of the problem with #81230. There, the
DB_File code, under -T, created a tainted tied object. Accessing the
object (within FETCH as it happens), caused mg_get() to be invoked on the
object (due to the taint magic), and thus extend the life of the object.
This then caused c<untie %h if $h{k}> to give the warning
    untie attempted while 1 inner references still exist.
This only became noticeable after efaf36747029c85b4d8825318cb4d485a0bb350e,
which stopped wrapping magic method calls in SAVETMPS/FREETMPS.

The second issue issue that this protection only applies to mg_get();
functions like mg_set() can still segfault if the SV is deleted.

This commit fixes both problems as follows:

First, the protection mechanism is moved out of mg_get() and into
save_magic() / restore_magic(), so that it protects more things.
Secondly, the protection is now:

* in save_magic(), SvREFCNT_inc() the SV, thus protecting it from being
  freed during FETCH (or whatever)

* in restore_magic(), SvREFCNT_dec() the SV, undoing the protection
  without extending the life of the SV, *except* if the refcount is
  1 (ie FETCH tried to free it), then push it on the mortals stack
  to extend it life a bit so our callers wont choke on it.
mg.c
t/op/taint.t
t/op/tie.t