Skip Menu |
Report information
Id: 81230
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: mmartinec <Mark.Martinec [at] ijs.si>
Cc:
AdminCc:

Operating System: freebsd
PatchStatus: (no value)
Severity: low
Type:
Perl Version: 5.13.8
Fixed In: (no value)



CC: Mark.Martinec [...] ijs.si
Subject: With a -T switch: untie attempted while 1 inner references still exist
Date: Fri, 24 Dec 2010 03:23:26 +0100 (CET)
To: perlbug [...] perl.org
From: Mark.Martinec [...] ijs.si
Download (untitled) / with headers
text/plain 3.1k
This is a bug report for perl from Mark.Martinec@ijs.si, generated with the help of perlbug 1.39 running under perl 5.13.8. ----------------------------------------------------------------- [Please describe your issue here] 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. [Please do not change anything below this line] ----------------------------------------------------------------- --- 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
Subject: Re: [perl #81230] With a -T switch: untie attempted while 1 inner references still exist
Date: Fri, 24 Dec 2010 17:00:30 +0000
To: perl5-porters [...] perl.org
From: Nicholas Clark <nick [...] ccl4.org>
Download (untitled) / with headers
text/plain 2.6k
On Thu, Dec 23, 2010 at 06:23:54PM -0800, Mark Martinec wrote: Thanks for the report. Show quoted text
> 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 efaf36747029c85b4d8825318cb4d485a0bb350e 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
Subject: Re: [perl #81230] With a -T switch: untie attempted while 1 inner references still exist
Date: Thu, 30 Dec 2010 10:55:34 +0000
To: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 5.1k
On Fri, Dec 24, 2010 at 05:00:30PM +0000, Nicholas Clark wrote: Show quoted text
> 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 efaf36747029c85b4d8825318cb4d485a0bb350e > 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 8985fe98dcc5c0af2fadeac15dfbc13f553ee7fc 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 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. 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


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org