With a -T switch: untie attempted while 1 inner references still exist

[p5pRT]

Migrated from rt.perl.org#81230 (status was 'resolved')

Searchable as RT81230$

[p5pRT]

From Mark.Martinec@ijs.si

Created by Mark.Martinec@ijs.si

Switching from perl 5.12.2 to 5.13.8 I noticed SpamAssassin's
self-test producing warnings. A test case distills down to​:

$ perl -Twe 'use DB_File; my %db;
  tie(%db,"DB_File","0.dat") or die $!;
  $db{"k"}="x"; untie %db if $db{"k"}'

produces​:

  untie attempted while 1 inner references still exist at -e line 1.

Leaving out the -T switch avoids the problem!

I'm not sure whether this is an issue with a module DB_File
or with core tie handling. I don't see an error in usage.
Everything else seems to be running fine.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.13.8:

Configured by mark at Thu Dec 23 19:15:09 CET 2010.

Summary of my perl5 (revision 5 version 13 subversion 8) configuration:
   
  Platform:
    osname=freebsd, osvers=7.2-release-p2, archname=amd64-freebsd
    uname='freebsd dorothy.ijs.si 7.2-release-p2 freebsd 7.2-release-p2 #0: wed jul 15 15:45:26 cest 2009 lesi@dorothy.ijs.si:usrobjusrsrcsysdorothy amd64 '
    config_args=''
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
    optimize='-O',
    cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.2.1 20070719  [FreeBSD]', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags ='-Wl,-E  -fstack-protector -L/usr/local/lib'
    libpth=/usr/lib /usr/local/lib
    libs=-lgdbm -lm -lcrypt -lutil -lc
    perllibs=-lm -lcrypt -lutil -lc
    libc=, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' '
    cccdlflags='-DPIC -fPIC', lddlflags='-shared  -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.13.8:
    /usr/local/lib/perl5/site_perl/5.13.8/amd64-freebsd
    /usr/local/lib/perl5/site_perl/5.13.8
    /usr/local/lib/perl5/5.13.8/amd64-freebsd
    /usr/local/lib/perl5/5.13.8
    /usr/local/lib/perl5/site_perl
    .


Environment for perl 5.13.8:
    HOME=/root
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/bin:/usr/local/sbin:/bin:/sbin:/usr/bin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/bash

[p5pRT]

From @nwc10

On Thu, Dec 23, 2010 at 06​:23​:54PM -0800, Mark Martinec wrote​:

Thanks for the report.

Switching from perl 5.12.2 to 5.13.8 I noticed SpamAssassin's
self-test producing warnings. A test case distills down to​:

$ perl -Twe 'use DB_File; my %db;
tie(%db,"DB_File","0.dat") or die $!;
$db{"k"}="x"; untie %db if $db{"k"}'

produces​:

untie attempted while 1 inner references still exist at -e line 1.

Leaving out the -T switch avoids the problem!

I'm not sure whether this is an issue with a module DB_File
or with core tie handling. I don't see an error in usage.
Everything else seems to be running fine.

I don't know either. I ran git bisect with this​:

#!/bin/sh
git clean -dxf
touch .patchnum
touch .sha1
touch unpushed.h
# If you can use ccache, add -Dcc=ccache\ gcc -Dld=gcc to the Configure line
# if Encode is not needed for the test, you can speed up the bisect by
# excluding it from the runs with -Dnoextensions=Encode
sh Configure -des -Dusedevel -Uusethreads -Doptimize="-g" -Dcc=ccache\ gcc -Dld=gcc -Dnoextensions=IPC/SysV\ Encode
test -f config.sh || exit 125
# Correct makefile for newer GNU gcc
perl -ni -we 'print unless /<(?​:built-in|command)/' makefile x2p/makefile
# if you just need miniperl, replace test_prep with miniperl
make -j5 test_prep
[ -x ./perl ] || exit 125
./perl -Ilib -Twe '$SIG{__WARN__} = sub {die @​_}; use DB_File; my %db; tie(%db,"DB_File","0.dat") or die $!; $db{"k"}="x"; untie %db if $db{"k"}'
ret=$?
[ $ret -gt 127 ] && ret=127
git clean -dxf
exit $ret

and it identifies the behaviour change as being due to this commit​:

commit efaf367
Author​: David Mitchell <davem@​iabyn.com>
Date​: Sun Apr 25 00​:56​:32 2010 +0100

  add Perl_magic_methcall
 
  Add a new function that wraps the setup needed to call a magic method like
  FETCH (the existing S_magic_methcall function has been renamed
  S_magic_methcall1).
 
  There is one functional change, done mainly to allow for a single clean
  wrapper function, and that is that the method calls are no longer wrapped
  with SAVETMPS/FREETMPS. Previously only about half of them had this, so
  some relied on the caller to free, some didn't. At least we're consistent
  now. Doing it this way is necessary because otherwise magic_methcall()
  can't return an SV (eg for POP) because it'll be a temp and get freed by
  FREETMPS before it gets returned. So you'd have to copy everything, which
  would slow things down.

I tried substituting DB_File in your example with Tie​::StdHash and there is
no warning. So I'm wondering if it's something to do with the implementation
of DB_File. I'm afraid that I don't have any insight into this.

Nicholas Clark

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @iabyn

On Fri, Dec 24, 2010 at 05​:00​:30PM +0000, Nicholas Clark wrote​:

On Thu, Dec 23, 2010 at 06​:23​:54PM -0800, Mark Martinec wrote​:

Thanks for the report.

Switching from perl 5.12.2 to 5.13.8 I noticed SpamAssassin's
self-test producing warnings. A test case distills down to​:

$ perl -Twe 'use DB_File; my %db;
tie(%db,"DB_File","0.dat") or die $!;
$db{"k"}="x"; untie %db if $db{"k"}'

produces​:

untie attempted while 1 inner references still exist at -e line 1.

Leaving out the -T switch avoids the problem!

I'm not sure whether this is an issue with a module DB_File
or with core tie handling. I don't see an error in usage.
Everything else seems to be running fine.

I don't know either. I ran git bisect with this​:

#!/bin/sh
git clean -dxf
touch .patchnum
touch .sha1
touch unpushed.h
# If you can use ccache, add -Dcc=ccache\ gcc -Dld=gcc to the Configure line
# if Encode is not needed for the test, you can speed up the bisect by
# excluding it from the runs with -Dnoextensions=Encode
sh Configure -des -Dusedevel -Uusethreads -Doptimize="-g" -Dcc=ccache\ gcc -Dld=gcc -Dnoextensions=IPC/SysV\ Encode
test -f config.sh || exit 125
# Correct makefile for newer GNU gcc
perl -ni -we 'print unless /<(?​:built-in|command)/' makefile x2p/makefile
# if you just need miniperl, replace test_prep with miniperl
make -j5 test_prep
[ -x ./perl ] || exit 125
./perl -Ilib -Twe '$SIG{__WARN__} = sub {die @​_}; use DB_File; my %db; tie(%db,"DB_File","0.dat") or die $!; $db{"k"}="x"; untie %db if $db{"k"}'
ret=$?
[ $ret -gt 127 ] && ret=127
git clean -dxf
exit $ret

and it identifies the behaviour change as being due to this commit​:

commit efaf367
Author​: David Mitchell <davem@​iabyn.com>
Date​: Sun Apr 25 00​:56​:32 2010 +0100

add Perl\_magic\_methcall

Add a new function that wraps the setup needed to call a magic method like
FETCH \(the existing S\_magic\_methcall function has been renamed
S\_magic\_methcall1\)\.

There is one functional change\, done mainly to allow for a single clean
wrapper function\, and that is that the method calls are no longer wrapped
with SAVETMPS/FREETMPS\. Previously only about half of them had this\, so
some relied on the caller to free\, some didn't\. At least we're consistent
now\. Doing it this way is necessary because otherwise magic\_methcall\(\)
can't return an SV \(eg for POP\) because it'll be a temp and get freed by
FREETMPS before it gets returned\. So you'd have to copy everything\, which
would slow things down\.

I tried substituting DB_File in your example with Tie​::StdHash and there is
no warning. So I'm wondering if it's something to do with the implementation
of DB_File. I'm afraid that I don't have any insight into this.

It's indirectly due to DB_File tainting its tied object. It can be
reproduced in plain perl by doing something similar. Fixed by the
following commit​:

commit 8985fe9
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Thu Dec 30 10​:32​:44 2010 +0000
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Thu Dec 30 10​:51​:45 2010 +0000

  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 efaf367,
  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.

Affected files ...
 
  M mg.c
  M t/op/taint.t
  M t/op/tie.t

--
Red sky at night - gerroff my land!
Red sky at morning - gerroff my land!
  -- old farmers' sayings #14

[p5pRT]

@iabyn - Status changed from 'open' to 'resolved'