Explorar o código

http2/hpack: fix memory leak in headerFieldTable lookup maps

An earlier performance change, https://golang.org/cl/37406,
made headerFieldTable lookups O(1). The entries in the maps used to perform
these lookups were not evicted along with the original field elements, however
causing a gradual memory leak. This is most apparent when the headers have
highly variable content such as request IDs or timings.

Fixes golang/go#19756

Change-Id: Icdb51527eb671925216350ada15f2a1336ea3158
Reviewed-on: https://go-review.googlesource.com/38781
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Chris Roche %!s(int64=8) %!d(string=hai) anos
pai
achega
05d3205156
Modificáronse 2 ficheiros con 28 adicións e 2 borrados
  1. 2 2
      http2/hpack/tables.go
  2. 26 0
      http2/hpack/tables_test.go

+ 2 - 2
http2/hpack/tables.go

@@ -69,10 +69,10 @@ func (t *headerFieldTable) evictOldest(n int) {
 		f := t.ents[k]
 		f := t.ents[k]
 		id := t.evictCount + uint64(k) + 1
 		id := t.evictCount + uint64(k) + 1
 		if t.byName[f.Name] == id {
 		if t.byName[f.Name] == id {
-			t.byName[f.Name] = 0
+			delete(t.byName, f.Name)
 		}
 		}
 		if p := (pairNameValue{f.Name, f.Value}); t.byNameValue[p] == id {
 		if p := (pairNameValue{f.Name, f.Value}); t.byNameValue[p] == id {
-			t.byNameValue[p] = 0
+			delete(t.byNameValue, p)
 		}
 		}
 	}
 	}
 	copy(t.ents, t.ents[n:])
 	copy(t.ents, t.ents[n:])

+ 26 - 0
http2/hpack/tables_test.go

@@ -89,6 +89,32 @@ func TestHeaderFieldTable(t *testing.T) {
 	}
 	}
 }
 }
 
 
+func TestHeaderFieldTable_LookupMapEviction(t *testing.T) {
+	table := &headerFieldTable{}
+	table.init()
+	table.addEntry(pair("key1", "value1-1"))
+	table.addEntry(pair("key2", "value2-1"))
+	table.addEntry(pair("key1", "value1-2"))
+	table.addEntry(pair("key3", "value3-1"))
+	table.addEntry(pair("key4", "value4-1"))
+	table.addEntry(pair("key2", "value2-2"))
+
+	// evict all pairs
+	table.evictOldest(table.len())
+
+	if l := table.len(); l > 0 {
+		t.Errorf("table.len() = %d, want 0", l)
+	}
+
+	if l := len(table.byName); l > 0 {
+		t.Errorf("len(table.byName) = %d, want 0", l)
+	}
+
+	if l := len(table.byNameValue); l > 0 {
+		t.Errorf("len(table.byNameValue) = %d, want 0", l)
+	}
+}
+
 func TestStaticTable(t *testing.T) {
 func TestStaticTable(t *testing.T) {
 	fromSpec := `
 	fromSpec := `
           +-------+-----------------------------+---------------+
           +-------+-----------------------------+---------------+