From 55b08e67d41a828c57623a3ebb715a8547b16eb7 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Fri, 25 Jan 2019 21:57:26 -0800
Subject: [PATCH] context: Avoid leaking ClassLoader through ThreadLocal

---
 .../io/grpc/ThreadLocalContextStorage.java    | 14 ++++++-
 .../grpc/ThreadLocalContextStorageTest.java   | 40 +++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 context/src/test/java/io/grpc/ThreadLocalContextStorageTest.java

diff --git a/context/src/main/java/io/grpc/ThreadLocalContextStorage.java b/context/src/main/java/io/grpc/ThreadLocalContextStorage.java
index ab734e292e..bab00cd92e 100644
--- a/context/src/main/java/io/grpc/ThreadLocalContextStorage.java
+++ b/context/src/main/java/io/grpc/ThreadLocalContextStorage.java
@@ -46,7 +46,19 @@ final class ThreadLocalContextStorage extends Context.Storage {
       log.log(Level.SEVERE, "Context was not attached when detaching",
           new Throwable().fillInStackTrace());
     }
-    doAttach(toRestore);
+    if (toRestore != Context.ROOT) {
+      localContext.set(toRestore);
+    } else {
+      // Avoid leaking our ClassLoader via ROOT if this Thread is reused across multiple
+      // ClassLoaders, as is common for Servlet Containers. The ThreadLocal is weakly referenced by
+      // the Thread, but its current value is strongly referenced and only lazily collected as new
+      // ThreadLocals are created.
+      //
+      // Use set(null) instead of remove() since remove() deletes the entry which is then re-created
+      // on the next get() (because of initialValue() handling). set(null) has same performance as
+      // set(toRestore).
+      localContext.set(null);
+    }
   }
 
   @Override
diff --git a/context/src/test/java/io/grpc/ThreadLocalContextStorageTest.java b/context/src/test/java/io/grpc/ThreadLocalContextStorageTest.java
new file mode 100644
index 0000000000..3ab4ed49cc
--- /dev/null
+++ b/context/src/test/java/io/grpc/ThreadLocalContextStorageTest.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2019 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class ThreadLocalContextStorageTest {
+  private static final Context.Key<Object> KEY = Context.key("test-key");
+  private ThreadLocalContextStorage storage = new ThreadLocalContextStorage();
+
+  @Test
+  public void detach_threadLocalClearedOnRoot() {
+    Context context = Context.ROOT.withValue(KEY, new Object());
+    Context old = storage.doAttach(context);
+    assertThat(old).isNull();
+    assertThat(storage.current()).isSameAs(context);
+    // Users see nulls converted to ROOT, so they will pass non-null as the "old" value
+    storage.detach(context, Context.ROOT);
+    assertThat(storage.current()).isNull();
+  }
+}