develooper Front page | perl.perl5.porters | Postings from July 2009

[PATCH] Restore original order of HV free

Thread Next
From:
Sam Vilain
Date:
July 29, 2009 22:22
Subject:
[PATCH] Restore original order of HV free
Message ID:
4A712BA5.1020803@vilain.net
In change 26550 (commit 7440661e26), a minor refactoring reversed the
order of freeing entries in HV arrays.  It was not the stated
intention of the change to alter this behaviour; it seems to be done
merely to avoid the use of an extra temporary variable. It can cause
some programs which set up complicated pipelines using a mix of open
FH, "|-", pipe() and fork() to deadlock where 5.8.x did not.
Restoring the original order of the free works around the problem.

Signed-off-by: Sam Vilain <sam@vilain.net>
---
 For maint-5.10; this is a 5.8.x to 5.10.0 regression.

 My theory is that by starting at the end of the HvARRAY and working
 backwards, the gremlins can see the destructor coming and run away.
 Starting at the beginning of the HvARRAY takes their feet first and
 therefore stops them from escaping.

 The program which it broke is apt-repository-simple, see
 http://git.catalyst.net.nz/gw?p=apt-repository-simple.git;a=blob;f=ars-update;hb=1d9e3f28#l241
 for the start of the section of code in question. 

 I'll be whittling that program down to a test case worthy of
 inclusion soon; this is to get people thinking about why the heck
 destruction is so sensitive to such a subtle under-the-hood change.

 hv.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hv.c b/hv.c
index 5d0f2f8..4fdac82 100644
--- a/hv.c
+++ b/hv.c
@@ -1667,7 +1667,8 @@ S_hfreeentries(pTHX_ HV *hv)
 	   it's the original array.  (Hopefully there will only be 1 time
 	   round) */
 	HE ** const array = HvARRAY(hv);
-	I32 i = HvMAX(hv);
+	I32 max = HvMAX(hv);
+	I32 i = 0;
 
 	/* Because we have taken xhv_name out, the only allocated pointer
 	   in the aux structure that might exist is the backreference array.
@@ -1748,7 +1749,7 @@ S_hfreeentries(pTHX_ HV *hv)
 		entry = HeNEXT(entry);
 		hv_free_ent(hv, oentry);
 	    }
-	} while (--i >= 0);
+	} while (++i <= max);
 
 	/* As there are no allocated pointers in the aux structure, it's now
 	   safe to free the array we just cleaned up, if it's not the one we're
-- 
1.5.6.5


Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About