finagle-base-http: change default header map impl

Problem / Solution

This patch changes the default header map implementation
for finagle-http. This is the remainder of this patch:
https://github.com/twitter/finagle/pull/805/files

Differential Revision: https://phabricator.twitter.biz/D394981

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/DefaultHeaderMap.scala

@@ -3,5 +3,5 @@ package com.twitter.finagle.http.headers
 import com.twitter.finagle.http.HeaderMap
 
 object DefaultHeaderMap {
-  def apply(headers: (String, String)*): HeaderMap = HashBackedHeaderMap(headers: _*)
+  def apply(headers: (String, String)*): HeaderMap = JTreeMapBackedHeaderMap(headers: _*)
 }

finagle-base-http/src/main/scala/com/twitter/finagle/http/headers/JTreeMapBackedHeaderMap.scala

@@ -2,7 +2,6 @@ package com.twitter.finagle.http.headers
 
 import com.twitter.finagle.http.HeaderMap
 import scala.annotation.tailrec
-import java.util.function.BiConsumer
 
 /**
  * Mutable, thread-safe [[HeaderMap]] implementation, backed by
@@ -21,15 +20,16 @@ final private class JTreeMapBackedHeaderMap extends HeaderMap {
   private[this] val underlying: java.util.TreeMap[String, Header.Root] =
     new java.util.TreeMap[String, Header.Root](JTreeMapBackedHeaderMap.sharedComparator)
 
-  private def foreachConsumer[U](f: ((String, String)) => U): BiConsumer[String, Header.Root] =
-    new BiConsumer[String, Header.Root]() {
-      def accept(key: String, header: Header.Root): Unit = header.iterator.foreach(
-        nv => f((nv.name, nv.value))
-      )
+  override def foreach[U](f: ((String, String)) => U): Unit = {
+    // We need to copy to a new iterator before calling the
+    // user defined functions `f` since they can add/remove
+    // to the header map which would invalidate the iterator
+    // on the underlying TreeMap and cause a
+    // ConcurrentModificationException.
+    val iter = copyHeaders
+    iter.foreach { root =>
+      root.iterator.foreach(nv => f((nv.name, nv.value)))
     }
-
-  override def foreach[U](f: ((String, String)) => U): Unit = underlying.synchronized {
-    underlying.forEach(foreachConsumer(f))
   }
 
   // ---- HeaderMap -----