From 05d3205156cb1bdc096578d6457fa161cd950aa2 Mon Sep 17 00:00:00 2001
From: Chris Roche <rodaine@gmail.com>
Date: Tue, 28 Mar 2017 17:59:59 -0700
Subject: [PATCH] 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>
---
 http2/hpack/tables.go      |  4 ++--
 http2/hpack/tables_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/http2/hpack/tables.go b/http2/hpack/tables.go
index 87015924..31bd5a55 100644
--- a/http2/hpack/tables.go
+++ b/http2/hpack/tables.go
@@ -69,10 +69,10 @@ func (t *headerFieldTable) evictOldest(n int) {
 		f := t.ents[k]
 		id := t.evictCount + uint64(k) + 1
 		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 {
-			t.byNameValue[p] = 0
+			delete(t.byNameValue, p)
 		}
 	}
 	copy(t.ents, t.ents[n:])
diff --git a/http2/hpack/tables_test.go b/http2/hpack/tables_test.go
index 7f40d9a4..d963f363 100644
--- a/http2/hpack/tables_test.go
+++ b/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) {
 	fromSpec := `
           +-------+-----------------------------+---------------+