Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DESTROY change in 5.8.0? #6023

Closed
p5pRT opened this issue Oct 21, 2002 · 7 comments
Closed

DESTROY change in 5.8.0? #6023

p5pRT opened this issue Oct 21, 2002 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 21, 2002

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

Searchable as RT18038$

@p5pRT
Copy link
Author

p5pRT commented Oct 21, 2002

From wayned@users.sourceforge.net

In earlier perl versions (such as 5.6.1), when an IO​::ScalarArray object
went out of scope, it was destroyed. It appears that 5.8.0 changes this
behavior due to the "tie *$self, $class, $self;" call that this class
makes in its new() subroutine. This means that old perl modules (such
as MIME​::Parser) can leak like a sieve whenever it uses a temporary
IO​::ScalarArray object (because the DESTROY calls don't happen until the
program exits).

For example, run this script under 5.8.0 and compare it to an older
perl. Under 5.8.0 all the DESTROY calls happen at the end, when they
used to happen after every function call. Uncommenting the "untie" line
restores the old behavior.

#!/usr/bin/perl
use strict;
use IO​::ScalarArray;

for (my $i = 0; $i < 20; $i++) {
  &hmm;
  print "$i\n";
}

sub hmm
{
  my @​data;
  my $tmp = IO​::ScalarArray->new(\@​data);
  #untie *$tmp; # Uncomment this to restore the old behavior.
}

I'm curious if this is an undesired change that will be fixed or if
program authors will need to change all their code that uses
IO​::ScalarArray (and any other hidden-tied classes like it)?

FYI​: I'm running Mandrake Linux 9.0 with the supplied perl 5.8.0.

....wayne..

@p5pRT
Copy link
Author

p5pRT commented Nov 1, 2002

From @ysth

On 21 Oct 2002 11​:34​:24 -0000, Wayne Davison wrote​:

In earlier perl versions (such as 5.6.1), when an IO​::ScalarArray object
went out of scope, it was destroyed. It appears that 5.8.0 changes this
behavior due to the "tie *$self, $class, $self;" call that this class
makes in its new() subroutine. This means that old perl modules (such
as MIME​::Parser) can leak like a sieve whenever it uses a temporary
IO​::ScalarArray object (because the DESTROY calls don't happen until the
program exits).

I'm curious if this is an undesired change that will be fixed or if
program authors will need to change all their code that uses
IO​::ScalarArray (and any other hidden-tied classes like it)?

It was undesired but introduced in fixing a different bug. The
adverse behaviour was not noticed until just before 5.8.0 was
released. We made several attempts to fix it at the eleventh hour
(with Jarkko chiming "Remember, at some point WE MUST STOP PATCHING.")
but failed. I also wasn't able to come up with an acceptable workaround.
You'll find this documented in perldelta/Self-tying problems​:

  A change to self-tying of globs has caused them to be recursively
  referenced (see​: "Two-Phased Garbage Collection" in perlobj). You will
  now need an explicit untie to destroy a self-tied glob. This behaviour
  may be fixed at a later date.

Thanks for bringing this up again. I'll take another stab at it
(but that doesn't help for 5.8.0).

@p5pRT
Copy link
Author

p5pRT commented Nov 4, 2002

From @ysth

On Mon, 28 Oct 2002 20​:27​:27 -0800, sthoenna@​efn.org wrote​:

On 21 Oct 2002 11​:34​:24 -0000, Wayne Davison wrote​:

In earlier perl versions (such as 5.6.1), when an IO​::ScalarArray object
went out of scope, it was destroyed. It appears that 5.8.0 changes this
behavior due to the "tie *$self, $class, $self;" call that this class
makes in its new() subroutine. This means that old perl modules (such
as MIME​::Parser) can leak like a sieve whenever it uses a temporary
IO​::ScalarArray object (because the DESTROY calls don't happen until the
program exits).

Thanks for bringing this up again. I'll take another stab at it
(but that doesn't help for 5.8.0).

I think I've got it (at the cost of breaking (but not with coredumps!)
a more pathological case--see new TODO test). With this patch,
IO​::Scalar and IO​::ScalarArray objects no longer leak.
All perl & IO-stringy tests pass.

Inline Patch
--- perl/sv.c.orig	Thu Oct 31 12:32:26 2002
+++ perl/sv.c	Sun Nov  3 15:18:40 2002
@@ -26,7 +26,7 @@
 #ifdef PERL_COPY_ON_WRITE
 #define SV_COW_NEXT_SV(sv)	INT2PTR(SV *,SvUVX(sv))
 #define SV_COW_NEXT_SV_SET(current,next)	SvUVX(current) = PTR2UV(next)
-/* This is a pessamistic view. Scalar must be purely a read-write PV to copy-
+/* This is a pessimistic view. Scalar must be purely a read-write PV to copy-
    on-write.  */
 #define CAN_COW_MASK	(SVs_OBJECT|SVs_GMG|SVs_SMG|SVs_RMG|SVf_IOK|SVf_NOK| \
 			 SVf_POK|SVf_ROK|SVp_IOK|SVp_NOK|SVp_POK|SVf_FAKE| \
@@ -4631,8 +4631,7 @@ Perl_sv_magicext(pTHX_ SV* sv, SV* obj, 
        avoid incrementing the object refcount.
 
        Note we cannot do this to avoid self-tie loops as intervening RV must
-       have its REFCNT incremented to keep it in existence - instead we could
-       special case them in sv_free() -- NI-S
+       have its REFCNT incremented to keep it in existence.
 
     */
     if (!obj || obj == sv ||
@@ -4649,6 +4648,21 @@ Perl_sv_magicext(pTHX_ SV* sv, SV* obj, 
 	mg->mg_obj = SvREFCNT_inc(obj);
 	mg->mg_flags |= MGf_REFCOUNTED;
     }
+
+    /* Normal self-ties simply pass a null object, and instead of
+       using mg_obj directly, use the SvTIED_obj macro to produce a
+       new RV as needed.  For glob "self-ties", we are tieing the PVIO
+       with an RV obj pointing to the glob containing the PVIO.  In
+       this case, to avoid a reference loop, we need to weaken the
+       reference.
+    */
+
+    if (how == PERL_MAGIC_tiedscalar && SvTYPE(sv) == SVt_PVIO &&
+        obj && SvROK(obj) && GvIO(SvRV(obj)) == (IO*)sv)
+    {
+      sv_rvweaken(obj);
+    }
+
     mg->mg_type = how;
     mg->mg_len = namlen;
     if (name) {
--- perl/t/op/tie.t.orig	Sun Nov  3 15:36:00 2002
+++ perl/t/op/tie.t	Sun Nov  3 15:57:58 2002
@@ -183,7 +183,7 @@
 EXPECT
 ########
 
-# TODO Allowed glob self-ties
+# Allowed glob self-ties
 my $destroyed = 0;
 my $printed   = 0;
 sub Self2::TIEHANDLE { bless $_[1], $_[0] }
@@ -204,12 +204,31 @@
 my $destroyed = 0;
 sub Self3::TIEHANDLE { bless $_[1], $_[0] }
 sub Self3::DESTROY   { $destroyed = 1; }
+sub Self3::PRINT     { $printed = 1; }
 {
     use Symbol 'geniosym';
     my $c = geniosym;
     tie *$c, 'Self3', $c;
+    print $c 'Hello';
 }
+die "self-tied IO not PRINTed" unless $printed == 1;
 die "self-tied IO not DESTROYed" unless $destroyed == 1;
+EXPECT
+########
+
+# TODO IO "self-tie" via TEMP glob
+my $destroyed = 0;
+sub Self3::TIEHANDLE { bless $_[1], $_[0] }
+sub Self3::DESTROY   { $destroyed = 1; }
+sub Self3::PRINT     { $printed = 1; }
+{
+    use Symbol 'geniosym';
+    my $c = geniosym;
+    tie *$c, 'Self3', \*$c;
+    print $c 'Hello';
+}
+die "IO tied to TEMP glob not PRINTed" unless $printed == 1;
+die "IO tied to TEMP glob not DESTROYed" unless $destroyed == 1;
 EXPECT
 ########
 
End of Patch.

@p5pRT
Copy link
Author

p5pRT commented Nov 7, 2002

From @hvds

sthoenna@​efn.org (Yitzchak Scott-Thoennes) wrote​:
:On Mon, 28 Oct 2002 20​:27​:27 -0800, sthoenna@​efn.org wrote​:
:>On 21 Oct 2002 11​:34​:24 -0000, Wayne Davison wrote​:
:>>In earlier perl versions (such as 5.6.1), when an IO​::ScalarArray object
:>>went out of scope, it was destroyed. It appears that 5.8.0 changes this
:>>behavior due to the "tie *$self, $class, $self;" call that this class
:>>makes in its new() subroutine.
[...]
:I think I've got it (at the cost of breaking (but not with coredumps!)
:a more pathological case--see new TODO test). With this patch,
:IO​::Scalar and IO​::ScalarArray objects no longer leak.

Thanks, applied as #18121.

Is there any particular reason the two op/tie.t tests declare and
initialise $destroyed but not $printed?

Hugo

@p5pRT
Copy link
Author

p5pRT commented Nov 8, 2002

@rspier - Status changed from 'new' to 'resolved'

@p5pRT
Copy link
Author

p5pRT commented Nov 27, 2002

From @jhi

Yitzchak considers the problem solved, I'm marking the problem ticket as such. (The fix will be in Perl 5.8.1).

@p5pRT
Copy link
Author

p5pRT commented Jan 11, 2003

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant