Project

General

Profile

Bug #16121 ยป 0002-Fix-freeing-and-clearing-destination-hash-in-Hash.diff.txt

[PATCH 2/4] Fix freeing and clearing destination hash in Hash#initialize_copy - dylants (Dylan Thacker-Smith), 08/23/2019 07:55 PM

 
1
From 5550a28098602b8fe0a68ac2529ffe2695beb589 Mon Sep 17 00:00:00 2001
2
From: Dylan Thacker-Smith <Dylan.Smith@shopify.com>
3
Date: Fri, 23 Aug 2019 15:38:06 -0400
4
Subject: [PATCH 2/4] Fix freeing and clearing destination hash in
5
 Hash#initialize_copy
6

    
7
The code was assuming the state of the destination hash based on the
8
source hash for clearing any existing table on it. If these don't match,
9
then that can cause the old table to be leaked. This can be seen by
10
compiling hash.c with `#define HASH_DEBUG 1` and running the following
11
script, which will crash from a debug assertion.
12

    
13
```ruby
14
h = 9.times.map { |i| [i, i] }.to_h
15
h.send(:initialize_copy, {})
16
```
17
---
18
 hash.c | 10 ++++++++--
19
 1 file changed, 8 insertions(+), 2 deletions(-)
20

    
21
diff --git a/hash.c b/hash.c
22
index f91958ff0f..ee5dc0e004 100644
23
--- a/hash.c
24
+++ b/hash.c
25
@@ -2792,14 +2792,20 @@ rb_hash_initialize_copy(VALUE hash, VALUE hash2)
26
 
27
     if (hash == hash2) return hash;
28
 
29
+    if (RHASH_ST_TABLE_P(hash)) {
30
+        st_free_table(RHASH_ST_TABLE(hash));
31
+        RHASH_ST_CLEAR(hash);
32
+    }
33
+    else {
34
+        ar_free_and_clear_table(hash);
35
+    }
36
+
37
     if (RHASH_AR_TABLE_P(hash2)) {
38
-        if (RHASH_AR_TABLE_P(hash)) ar_free_and_clear_table(hash);
39
         ar_copy(hash, hash2);
40
         if (RHASH_AR_TABLE_SIZE(hash))
41
 	    rb_hash_rehash(hash);
42
     }
43
     else if (RHASH_ST_TABLE_P(hash2)) {
44
-        if (RHASH_ST_TABLE_P(hash)) st_free_table(RHASH_ST_TABLE(hash));
45
         RHASH_ST_TABLE_SET(hash, st_copy(RHASH_ST_TABLE(hash2)));
46
         if (RHASH_ST_TABLE(hash)->num_entries)
47
             rb_hash_rehash(hash);
48
-- 
49
2.21.0
50