From 85244737587c6651e8d467765eed23b172e60ad2 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Tue, 1 Feb 2011 17:44:36 -0800
Subject: [PATCH 01/43] appropriate names everywhere.

---
 ...Manager.scala => HttpServerConnectionLifecycleManager.scala} | 0
 ...pec.scala => HttpServerConnectionLifecycleManagerSpec.scala} | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename finagle-core/src/main/scala/com/twitter/finagle/http/{ConnectionLifecycleManager.scala => HttpServerConnectionLifecycleManager.scala} (100%)
 rename finagle-core/src/test/scala/com/twitter/finagle/http/{ConnectionLifecycleManagerSpec.scala => HttpServerConnectionLifecycleManagerSpec.scala} (98%)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionLifecycleManager.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala
similarity index 100%
rename from finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionLifecycleManager.scala
rename to finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionLifecycleManagerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
similarity index 98%
rename from finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionLifecycleManagerSpec.scala
rename to finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
index c8a9dcb267..d7f4546fad 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionLifecycleManagerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
@@ -9,7 +9,7 @@ import org.jboss.netty.channel._
 import org.jboss.netty.handler.codec.http._
 import org.jboss.netty.buffer.ChannelBuffers
 
-object ConnectionLifecycleManagerSpec extends Specification with Mockito {
+object HttpServerConnectionLifecycleManagerSpec extends Specification with Mockito {
   // > further tests
   //   - malformed requests/responses
   //   - methods other than GET

From 39cec78206195ba6bbd6b04b1d37cb2f3f4eeeb4 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Tue, 1 Feb 2011 17:45:26 -0800
Subject: [PATCH 02/43] remove spurious line.

---
 .../finagle/http/HttpServerConnectionLifecycleManagerSpec.scala | 2 --
 1 file changed, 2 deletions(-)

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
index d7f4546fad..79ea8ce27a 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
@@ -23,8 +23,6 @@ object HttpServerConnectionLifecycleManagerSpec extends Specification with Mocki
     val cFuture = new DefaultChannelFuture(c, false)
     me.getChannel returns c
 
-    new DefaultHttpResponse(HttpVersion.HTTP_1_0, HttpResponseStatus.OK)
-
     def makeRequest(version: HttpVersion, headers: (String, String)*) = {
       val request = new DefaultHttpRequest(version, HttpMethod.GET, "/")
       headers foreach { case (k, v) =>

From d1d8789b947451d25df4dcc3a97bee9dbeff04c6 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Tue, 1 Feb 2011 19:04:11 -0800
Subject: [PATCH 03/43] introduce a ClientConnectionManager for HTTP.

---
 .../com/twitter/finagle/builder/Http.scala    |   2 +-
 .../finagle/channel/ChannelService.scala      |   3 +
 .../http/ClientConnectionManager.scala        |  26 +++++
 .../finagle/http/ConnectionManager.scala      |  40 +++++++
 ...HttpServerConnectionLifecycleManager.scala |  64 -----------
 .../http/ServerConnectionManager.scala        |  39 +++++++
 ...Spec.scala => ConnectionManagerSpec.scala} | 107 ++++++++++++++----
 7 files changed, 194 insertions(+), 87 deletions(-)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/http/ClientConnectionManager.scala
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionManager.scala
 delete mode 100644 finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/http/ServerConnectionManager.scala
 rename finagle-core/src/test/scala/com/twitter/finagle/http/{HttpServerConnectionLifecycleManagerSpec.scala => ConnectionManagerSpec.scala} (60%)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
index c24f8239e9..c861c41758 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
@@ -31,7 +31,7 @@ class Http(compressionLevel: Int = 0) extends Codec[HttpRequest, HttpResponse] {
 
         pipeline.addLast(
           "connectionLifecycleManager",
-          new HttpServerConnectionLifecycleManager)
+          new ServerConnectionManager)
 
         pipeline
       }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
index e79a1c8a08..e3905431b0 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
@@ -72,6 +72,9 @@ class ChannelService[Req, Rep](channel: Channel, factory: ChannelServiceFactory[
   def apply(request: Req) = {
     val replyFuture = new Promise[Rep]
     if (currentReplyFuture.compareAndSet(null, replyFuture)) {
+      // XXX - listen on this future -- if it's a write failure, then
+      // issue a WriteException -- otherwise, the standard path.
+      
       Channels.write(channel, request)
       replyFuture
     } else {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/ClientConnectionManager.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/ClientConnectionManager.scala
new file mode 100644
index 0000000000..4d01792a1f
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/http/ClientConnectionManager.scala
@@ -0,0 +1,26 @@
+package com.twitter.finagle.http
+
+import org.jboss.netty.channel._
+import org.jboss.netty.handler.codec.http._
+
+import com.twitter.finagle.util.Conversions._
+
+class ClientConnectionManager extends SimpleChannelHandler {
+  private[this] val manager = new ConnectionManager
+
+  // Note that for HTTP requests without a content length, the
+  // underlying codec does the right thing (flushes the buffer until
+  // the connection has been closed).
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
+    super.messageReceived(ctx, e)
+
+    manager.observeMessage(e.getMessage)
+    if (manager.shouldClose)
+      e.getChannel.close()
+  }
+
+  override def writeRequested(ctx: ChannelHandlerContext, e: MessageEvent) {
+    manager.observeMessage(e.getMessage)
+    super.writeRequested(ctx, e)
+  }
+}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionManager.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionManager.scala
new file mode 100644
index 0000000000..5222aeb051
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/http/ConnectionManager.scala
@@ -0,0 +1,40 @@
+package com.twitter.finagle.http
+
+import org.jboss.netty.handler.codec.http._
+
+class ConnectionManager {
+  private[this] var isKeepAlive = false
+  private[this] var isIdle = true
+
+  def observeMessage(message: AnyRef) = synchronized {
+    message match {
+      case request: HttpRequest   => observeRequest(request)
+      case response: HttpResponse => observeResponse(response)
+      case chunk: HttpChunk       => observeChunk(chunk)
+      case _                      => isKeepAlive = false  // conservative
+    }
+  }
+
+  def observeRequest(request: HttpRequest) = synchronized {
+    isIdle = false
+    isKeepAlive = HttpHeaders.isKeepAlive(request)
+  }
+
+  def observeResponse(response: HttpResponse) = synchronized {
+    if (!response.isChunked && !response.containsHeader(HttpHeaders.Names.CONTENT_LENGTH))
+      isKeepAlive = false
+    else if (!HttpHeaders.isKeepAlive(response))
+      isKeepAlive = false
+
+    // If a response isn't chunked, then we're done with this request,
+    // and hence idle.
+    isIdle = !response.isChunked
+  }
+
+  def observeChunk(chunk: HttpChunk) = synchronized {
+    require(!isIdle)
+    isIdle = chunk.isLast
+  }
+
+  def shouldClose() = synchronized { isIdle && !isKeepAlive }
+}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala
deleted file mode 100644
index b8c14842f9..0000000000
--- a/finagle-core/src/main/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManager.scala
+++ /dev/null
@@ -1,64 +0,0 @@
-package com.twitter.finagle.http
-
-import org.jboss.netty.channel._
-import org.jboss.netty.handler.codec.http._
-
-import com.twitter.finagle.util.Conversions._
-
-/**
- * Deal with Http connection lifecycle wrt. keepalives, etc.
- *
- * See: http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html
- */
-
-// > TODO
-//   - should we support http/1.0 keepalives?
-//   - should there be a general "connection dirtying" mechanism?
-//   - keepalive policies (eg. max idle time)
-
-class HttpServerConnectionLifecycleManager extends SimpleChannelHandler {
-  @volatile private[this] var isKeepAlive = false
-
-  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
-    // TODO: what happens in a upload situation here -- can we get
-    // chunked content?
-    e.getMessage match {
-      case request: HttpRequest =>
-        isKeepAlive = HttpHeaders.isKeepAlive(request)
-
-      case _ =>
-        isKeepAlive = false
-    }
-
-    super.messageReceived(ctx, e)
-  }
-
-  override def writeRequested(ctx: ChannelHandlerContext, e: MessageEvent) {
-    e.getMessage match {
-      case response: HttpResponse =>
-        if (!response.isChunked && (response.getHeader(HttpHeaders.Names.CONTENT_LENGTH) eq null))
-          isKeepAlive = false
-
-        // We respect server-side termination as well.
-        if (!HttpHeaders.isKeepAlive(response))
-          isKeepAlive = false
-
-      case _ =>
-        ()
-    }
-
-    if (!isKeepAlive) {
-      val shouldTerminate =
-        e.getMessage match {
-          case response: HttpResponse if !response.isChunked => true
-          case chunk: HttpChunk if chunk.isLast => true
-          case _ => false
-        }
-
-      if (shouldTerminate)
-        e.getFuture.close()
-    }
-
-    super.writeRequested(ctx, e)
-  }
-}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/ServerConnectionManager.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/ServerConnectionManager.scala
new file mode 100644
index 0000000000..6bef60ef2e
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/http/ServerConnectionManager.scala
@@ -0,0 +1,39 @@
+package com.twitter.finagle.http
+
+import org.jboss.netty.channel._
+import org.jboss.netty.handler.codec.http._
+
+import com.twitter.finagle.util.Conversions._
+
+/**
+ * Deal with Http connection lifecycle wrt. keepalives, etc.
+ *
+ * See: http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html
+ */
+
+// > TODO
+//   - should we support http/1.0 keepalives?
+//   - should there be a general "connection dirtying" mechanism?
+//   - keepalive policies (eg. max idle time)
+
+// TODO: "If either the client or the server sends the close token in
+// the Connection header, that request becomes the last one for the
+// connection."
+
+class ServerConnectionManager extends SimpleChannelHandler {
+  private[this] val manager = new ConnectionManager
+
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
+    manager.observeMessage(e.getMessage)
+    super.messageReceived(ctx, e)
+  }
+
+  override def writeRequested(ctx: ChannelHandlerContext, e: MessageEvent) {
+    manager.observeMessage(e.getMessage)
+
+    if (manager.shouldClose)
+      e.getFuture.close()
+
+    super.writeRequested(ctx, e)
+  }
+}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionManagerSpec.scala
similarity index 60%
rename from finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
rename to finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionManagerSpec.scala
index 79ea8ce27a..a3cf99a59f 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/http/HttpServerConnectionLifecycleManagerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/http/ConnectionManagerSpec.scala
@@ -9,39 +9,103 @@ import org.jboss.netty.channel._
 import org.jboss.netty.handler.codec.http._
 import org.jboss.netty.buffer.ChannelBuffers
 
-object HttpServerConnectionLifecycleManagerSpec extends Specification with Mockito {
+object ConnectionManagerSpec extends Specification with Mockito {
   // > further tests
   //   - malformed requests/responses
   //   - methods other than GET
   //   - 100/continue
 
-  "the HTTP connection lifecycle handler" should {
-    val handler = new HttpServerConnectionLifecycleManager
-    val me = mock[MessageEvent]
-    val c = mock[Channel]
-    val ctx = mock[ChannelHandlerContext]
-    val cFuture = new DefaultChannelFuture(c, false)
-    me.getChannel returns c
+  val me = mock[MessageEvent]
+  val c = mock[Channel]
+  val ctx = mock[ChannelHandlerContext]
+  val cFuture = new DefaultChannelFuture(c, false)
+  me.getChannel returns c
+
+  def makeRequest(version: HttpVersion, headers: (String, String)*) = {
+    val request = new DefaultHttpRequest(version, HttpMethod.GET, "/")
+    headers foreach { case (k, v) =>
+      request.setHeader(k, v)
+    }
+
+    request
+  }
+
+  def makeResponse(version: HttpVersion, headers: (String, String)*) = {
+    val response = new DefaultHttpResponse(version, HttpResponseStatus.OK)
+    headers foreach { case (k, v) =>
+      response.setHeader(k, v)
+    }
+
+    response
+  }
 
-    def makeRequest(version: HttpVersion, headers: (String, String)*) = {
-      val request = new DefaultHttpRequest(version, HttpMethod.GET, "/")
-      headers foreach { case (k, v) =>
-        request.setHeader(k, v)
-      }
 
-      request
+  "the client HTTP connection manager" should {
+    val handler = new ClientConnectionManager
+
+    def perform(request: HttpRequest, response: HttpResponse) {
+      me.getMessage returns request
+      me.getFuture returns cFuture
+      handler.writeRequested(ctx, me)
+
+      me.getMessage returns response
+      handler.messageReceived(ctx, me)
     }
 
-    def makeResponse(version: HttpVersion, headers: (String, String)*) = {
-      val response = new DefaultHttpResponse(version, HttpResponseStatus.OK)
-      headers foreach { case (k, v) =>
-        response.setHeader(k, v)
-      }
+    "not terminate regular http/1.1 connections" in {
+      perform(
+        makeRequest(HttpVersion.HTTP_1_1),
+        makeResponse(HttpVersion.HTTP_1_1, HttpHeaders.Names.CONTENT_LENGTH -> "1")
+      )
 
-      response
+      there was no(c).close()
     }
 
+    // Note: by way of the codec, this reply is already taken care of.
+    "terminate http/1.1 connections without content length" in {
+      perform(
+        makeRequest(HttpVersion.HTTP_1_1),
+        makeResponse(HttpVersion.HTTP_1_1)
+      )
 
+      there was one(c).close()
+    }
+
+    "terminate http/1.1 connections with Connection: close" in {
+      perform(
+        makeRequest(HttpVersion.HTTP_1_1, "Connection" -> "close"),
+        makeResponse(HttpVersion.HTTP_1_1)
+      )
+      
+      there was one(c).close()
+    }
+
+    "terminate chunked http/1.1 with Connection: close" in {
+      val request = makeRequest(HttpVersion.HTTP_1_1, "connection" -> "close")
+      val response = makeResponse(HttpVersion.HTTP_1_1)
+      response.setChunked(true)
+
+      perform(request, response)
+      there was no(c).close()
+
+      val chunk = new DefaultHttpChunk(
+        ChannelBuffers.copiedBuffer("content", Charset.forName("UTF-8")))
+
+      me.getMessage returns chunk
+      handler.messageReceived(ctx, me)
+      me.getMessage returns chunk
+      handler.messageReceived(ctx, me)
+
+      // The final chunk.
+      me.getMessage returns new DefaultHttpChunkTrailer
+      handler.messageReceived(ctx, me)
+
+      there was one(c).close()
+    }
+  }
+
+  "the server HTTP connection manager" should {
+    val handler = new ServerConnectionManager
     def perform(request: HttpRequest, response: HttpResponse) {
       me.getMessage returns request
       handler.messageReceived(ctx, me)
@@ -80,12 +144,11 @@ object HttpServerConnectionLifecycleManagerSpec extends Specification with Mocki
         makeResponse(HttpVersion.HTTP_1_1, HttpHeaders.Names.CONTENT_LENGTH -> "1")
       )
 
-      there was no(c).close()
       cFuture.setSuccess()   // write success
       there was no(c).close()
     }
 
-    "not terminate http/1.1 request with missing content-length (in the response)" in {
+    "terminate http/1.1 request with missing content-length (in the response)" in {
       perform(
         makeRequest(HttpVersion.HTTP_1_1),
         makeResponse(HttpVersion.HTTP_1_1)

From 0694dbed8829a99642d8785a64cd4df2b240ef90 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Tue, 1 Feb 2011 19:10:37 -0800
Subject: [PATCH 04/43] add the HTTP ClientConnetionManager do the default
 handler stack for HTTP clients.

---
 .../src/main/scala/com/twitter/finagle/builder/Http.scala    | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
index c861c41758..9d2fe73366 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
@@ -14,6 +14,11 @@ class Http(compressionLevel: Int = 0) extends Codec[HttpRequest, HttpResponse] {
         pipeline.addLast("httpCodec",        new HttpClientCodec())
         pipeline.addLast("httpDechunker",    new HttpChunkAggregator(10<<20))
         pipeline.addLast("httpDecompressor", new HttpContentDecompressor)
+
+        pipeline.addLast(
+          "connectionLifecycleManager",
+          new ClientConnectionManager)
+
         pipeline
       }
     }

From 2067d708f9505312383d8ce42f9d7dfb640ae11e Mon Sep 17 00:00:00 2001
From: Nick Kallen <nkallen@tallbro.local>
Date: Tue, 1 Feb 2011 21:18:52 -0800
Subject: [PATCH 05/43] janky backpressure implementation

---
 .../finagle/stream/ChannelToHttpChunk.scala   | 30 +++++++++----
 .../finagle/stream/HttpChunkToChannel.scala   | 44 +++++++++++++------
 .../twitter/finagle/stream/EndToEndSpec.scala | 31 +++++++------
 3 files changed, 66 insertions(+), 39 deletions(-)

diff --git a/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala b/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
index a60f55fab3..cf717a5775 100644
--- a/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
+++ b/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
@@ -1,10 +1,10 @@
 package com.twitter.finagle.stream
 
 import org.jboss.netty.buffer.ChannelBuffer
-import com.twitter.concurrent.{End, Value}
 import org.jboss.netty.handler.codec.http._
 import org.jboss.netty.channel._
 import java.util.concurrent.atomic.AtomicReference
+import com.twitter.concurrent.Observer
 
 /**
  * A Netty Channel Handler that adapts Twitter Channels to Netty Channels.
@@ -16,6 +16,8 @@ import java.util.concurrent.atomic.AtomicReference
 class ChannelToHttpChunk extends SimpleChannelDownstreamHandler {
   private[this] val channelRef =
     new AtomicReference[com.twitter.concurrent.Channel[ChannelBuffer]](null)
+  private[this] val observerRef =
+    new AtomicReference[Observer](null)
 
   override def writeRequested(ctx: ChannelHandlerContext, e: MessageEvent) = e.getMessage match {
     case channel: com.twitter.concurrent.Channel[ChannelBuffer] =>
@@ -23,21 +25,31 @@ class ChannelToHttpChunk extends SimpleChannelDownstreamHandler {
 
       val startMessage = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK)
       HttpHeaders.setHeader(startMessage, "Transfer-Encoding", "Chunked")
+
       val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
       Channels.write(ctx, ignoreOutcome, startMessage)
-      channel receive {
-        case Value(channelBuffer) =>
-          val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
-          Channels.write(ctx, ignoreOutcome, new DefaultHttpChunk(channelBuffer))
-        case End =>
-          val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
-          channelRef.set(null)
-          Channels.write(ctx, ignoreOutcome, new DefaultHttpChunkTrailer)
+      val observer = channel.respond(this) { channelBuffer =>
+        val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
+        Channels.write(ctx, ignoreOutcome, new DefaultHttpChunk(channelBuffer))
+      }
+      observerRef.set(observer)
+      channel.closes.respond { _ =>
+        val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
+        channelRef.set(null)
+        observerRef.set(null)
+        Channels.write(ctx, ignoreOutcome, new DefaultHttpChunkTrailer)
       }
     case _ =>
       throw new IllegalArgumentException("Expecting a Channel" + e.getMessage)
   }
 
+
+  override def setInterestOpsRequested(ctx: ChannelHandlerContext, e: ChannelStateEvent) = {
+    val observer = observerRef.get
+    if (ctx.getChannel.isWritable) observer.resume()
+    else observer.pause()
+  }
+
   override def closeRequested(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
     val channel = channelRef.get
     if (channel ne null) channel.close()
diff --git a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
index 5a61dc86ec..f9fa5917ce 100644
--- a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
+++ b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
@@ -1,35 +1,51 @@
 package com.twitter.finagle.stream
 
 import org.jboss.netty.handler.codec.http.{HttpChunkTrailer, HttpChunk, HttpMessage}
-import java.util.concurrent.atomic.AtomicReference
 import org.jboss.netty.buffer.ChannelBuffer
-import com.twitter.concurrent.Topic
 import org.jboss.netty.channel.{Channels, MessageEvent, ChannelHandlerContext, SimpleChannelUpstreamHandler}
+import java.util.concurrent.atomic.{AtomicInteger, AtomicReference}
+import com.twitter.concurrent.{Serialized, ChannelSource}
 
-class HttpChunkToChannel extends SimpleChannelUpstreamHandler {
-  private[this] val topicRef =
-    new AtomicReference[com.twitter.concurrent.Topic[ChannelBuffer]](null)
+/**
+ * Client handler for a streaming protocol.
+ */
+class HttpChunkToChannel extends SimpleChannelUpstreamHandler with Serialized {
+  private[this] val channelRef =
+    new AtomicReference[com.twitter.concurrent.ChannelSource[ChannelBuffer]](null)
 
   override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) = e.getMessage match {
     case message: HttpMessage =>
-      val topic = new Topic[ChannelBuffer]
-      require(topicRef.compareAndSet(null, topic),
+      val source = new ChannelSource[ChannelBuffer]
+      require(channelRef.compareAndSet(null, source),
         "Channel is already busy")
       ctx.getChannel.setReadable(false)
-      topic.onReceive {
+      source.responds.first.respond { _ =>
         if (!message.isChunked) {
-          topic.send(message.getContent)
-          topic.close()
-          topicRef.set(null)
+          source.send(message.getContent)
+          source.close()
+          channelRef.set(null)
         }
         ctx.getChannel.setReadable(true)
       }
-      Channels.fireMessageReceived(ctx, topic)
+      var pausedCount = 0
+      source.pauses.respond(this) { _ =>
+        serialized {
+          pausedCount += 1
+          if (pausedCount == 1) ctx.getChannel.setReadable(false)
+        }
+      }
+      source.resumes.respond(this) { _ =>
+        serialized {
+          pausedCount -= 1
+          if (pausedCount == 0) ctx.getChannel.setReadable(true)
+        }
+      }
+      Channels.fireMessageReceived(ctx, source)
     case trailer: HttpChunkTrailer =>
-      val topic = topicRef.getAndSet(null)
+      val topic = channelRef.getAndSet(null)
       topic.close()
     case chunk: HttpChunk =>
-      val topic = topicRef.get
+      val topic = channelRef.get
       topic.send(chunk.getContent)
   }
 }
\ No newline at end of file
diff --git a/finagle-stream/src/test/scala/com/twitter/finagle/stream/EndToEndSpec.scala b/finagle-stream/src/test/scala/com/twitter/finagle/stream/EndToEndSpec.scala
index d34c9f9fbd..4e8d29314c 100644
--- a/finagle-stream/src/test/scala/com/twitter/finagle/stream/EndToEndSpec.scala
+++ b/finagle-stream/src/test/scala/com/twitter/finagle/stream/EndToEndSpec.scala
@@ -6,23 +6,24 @@ import com.twitter.concurrent._
 import com.twitter.finagle.builder.{ClientBuilder, ServerBuilder}
 import org.jboss.netty.handler.codec.http.{HttpMethod, HttpVersion, DefaultHttpRequest, HttpRequest}
 import org.jboss.netty.buffer.{ChannelBuffers, ChannelBuffer}
-import com.twitter.util.{CountDownLatch, Future, RandomSocket}
+import com.twitter.util.{Future, RandomSocket}
+import com.twitter.concurrent._
 import com.twitter.conversions.time._
 import java.nio.charset.Charset
 
 object EndToEndSpec extends Specification {
-  class MyService(topic: Topic[ChannelBuffer]) extends Service[HttpRequest, Channel[ChannelBuffer]] {
+  class MyService(topic: ChannelSource[ChannelBuffer]) extends Service[HttpRequest, Channel[ChannelBuffer]] {
     def apply(request: HttpRequest) = Future.value(topic)
   }
 
   "Streams" should {
     "work" in {
       val address = RandomSocket()
-      val topic = new Topic[ChannelBuffer]
+      val channelSource = new ChannelSource[ChannelBuffer]
       val server = ServerBuilder()
         .codec(new Stream)
         .bindTo(address)
-        .build(new MyService(topic))
+        .build(new MyService(channelSource))
       val client = ClientBuilder()
         .codec(new Stream)
         .hosts(Seq(address))
@@ -32,32 +33,30 @@ object EndToEndSpec extends Specification {
 
       "writes from the server arrive on the client's channel" in {
         var result = ""
-        channel.foreach { channelBuffer =>
+        channel.respond(this) { channelBuffer =>
           result += channelBuffer.toString(Charset.defaultCharset)
         }
 
-        topic.send(ChannelBuffers.wrappedBuffer("1".getBytes))
-        topic.send(ChannelBuffers.wrappedBuffer("2".getBytes))
-        topic.send(ChannelBuffers.wrappedBuffer("3".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("1".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("2".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("3".getBytes))
 
-        topic.close()
-        channel.join()
+        channelSource.close()
 
         result mustEqual "123"
       }
 
       "writes from the server are queued before the client receives" in {
-        topic.send(ChannelBuffers.wrappedBuffer("1".getBytes))
-        topic.send(ChannelBuffers.wrappedBuffer("2".getBytes))
-        topic.send(ChannelBuffers.wrappedBuffer("3".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("1".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("2".getBytes))
+        channelSource.send(ChannelBuffers.wrappedBuffer("3".getBytes))
 
         var result = ""
-        channel.foreach { channelBuffer =>
+        channel.respond(this) { channelBuffer =>
           result += channelBuffer.toString(Charset.defaultCharset)
         }
 
-        topic.close()
-        channel.join()
+        channelSource.close()
 
         result mustEqual "123"
       }

From 3d1bc2dac7da0d4f46418e1e4608238dbc5e26c3 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Wed, 2 Feb 2011 11:11:46 -0800
Subject: [PATCH 06/43] propagate write errors in the ChannelService.

---
 .../com/twitter/finagle/Exceptions.scala      | 12 +++++++++
 .../finagle/channel/ChannelService.scala      | 26 ++++++++-----------
 .../finagle/channel/ChannelServiceSpec.scala  | 12 +++++++--
 3 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/Exceptions.scala b/finagle-core/src/main/scala/com/twitter/finagle/Exceptions.scala
index f84ffa8776..414551111c 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/Exceptions.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/Exceptions.scala
@@ -20,6 +20,18 @@ class WriteException(e: Throwable)          extends ChannelException {
   override def toString = "%s: %s".format(super.toString, e.toString)
 }
 
+object ChannelException {
+  def apply(cause: Throwable) = {
+    cause match {
+      case exc: ChannelException => exc
+      case _: java.net.ConnectException                    => new ConnectionFailedException
+      case _: java.nio.channels.UnresolvedAddressException => new ConnectionFailedException
+      case _: java.nio.channels.ClosedChannelException     => new ChannelClosedException
+      case e                                               => new UnknownChannelException(e)
+    }
+  }
+}
+
 // Service layer errors.
 class ServiceException       extends Exception
 class ServiceClosedException extends ServiceException
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
index e3905431b0..e09a69057f 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
@@ -4,7 +4,10 @@ import java.util.concurrent.atomic.AtomicReference
 import java.util.concurrent.LinkedBlockingQueue
 
 import org.jboss.netty.bootstrap.ClientBootstrap
-import org.jboss.netty.channel._
+import org.jboss.netty.channel.{
+  ChannelHandlerContext, MessageEvent, Channel, Channels,
+  SimpleChannelUpstreamHandler, ExceptionEvent,
+  ChannelStateEvent}
 
 import com.twitter.util.{Future, Promise, Return, Throw, Try}
 
@@ -53,16 +56,8 @@ class ChannelService[Req, Rep](channel: Channel, factory: ChannelServiceFactory[
       reply(Try { e.getMessage.asInstanceOf[Rep] })
     }
 
-    override def exceptionCaught(ctx: ChannelHandlerContext, e: ExceptionEvent) = {
-      val translated = e.getCause match {
-        case _: java.net.ConnectException                    => new ConnectionFailedException
-        case _: java.nio.channels.UnresolvedAddressException => new ConnectionFailedException
-        case _: java.nio.channels.ClosedChannelException     => new ChannelClosedException
-        case e                                               => new UnknownChannelException(e)
-      }
-
-      reply(Throw(translated))
-    }
+    override def exceptionCaught(ctx: ChannelHandlerContext, e: ExceptionEvent) =
+      reply(Throw(ChannelException(e.getCause)))
 
     override def channelClosed(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
       reply(Throw(new ChannelClosedException))
@@ -72,10 +67,11 @@ class ChannelService[Req, Rep](channel: Channel, factory: ChannelServiceFactory[
   def apply(request: Req) = {
     val replyFuture = new Promise[Rep]
     if (currentReplyFuture.compareAndSet(null, replyFuture)) {
-      // XXX - listen on this future -- if it's a write failure, then
-      // issue a WriteException -- otherwise, the standard path.
-      
-      Channels.write(channel, request)
+      Channels.write(channel, request) {
+        case Error(cause) =>
+          replyFuture.updateIfEmpty(Throw(new WriteException(ChannelException(cause))))
+        case _ => ()
+      }
       replyFuture
     } else {
       Future.exception(new TooManyConcurrentRequestsException)
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
index a6f76539fc..53606af12f 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
@@ -36,9 +36,17 @@ object ChannelServiceSpec extends Specification with Mockito {
       val future = service("hello")
       val eventCaptor = ArgumentCaptor.forClass(classOf[ChannelEvent])
       there was one(sink).eventSunk(Matchers.eq(pipeline), eventCaptor.capture)
-      
       eventCaptor.getValue must haveClass[DownstreamMessageEvent]
-      eventCaptor.getValue.asInstanceOf[DownstreamMessageEvent].getMessage must be_==("hello")
+      val messageEvent = eventCaptor.getValue.asInstanceOf[DownstreamMessageEvent]
+      
+      "propagate the correct message" in {
+        messageEvent.getMessage must be_==("hello")
+      }
+
+      "cause write errors if the downstream write fails" in {
+        messageEvent.getFuture.setFailure(new Exception("doh."))
+        future() must throwA(new WriteException(new Exception("doh.")))
+      }
     }
 
     "receive replies" in {

From a32e31ff98dfee1570512dcd28054fd58105e7c9 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Wed, 2 Feb 2011 11:18:25 -0800
Subject: [PATCH 07/43] simplify the SingletonFactory by using a FutureLatch.

---
 .../finagle/service/SingletonFactory.scala    | 21 ++++++-------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
index 9829c856c1..734c5f6a55 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
@@ -3,31 +3,22 @@ package com.twitter.finagle.service
 import com.twitter.util.Future
 
 import com.twitter.finagle.{Service, ServiceFactory}
+import com.twitter.finagle.util.FutureLatch
        
 class SingletonFactory[Req, Rep](service: Service[Req, Rep])
   extends ServiceFactory[Req, Rep]
 {
-  private[this] var outstandingInstances = 0
-  private[this] var needsRelease = false
+  private[this] var latch = new FutureLatch
 
-  def make() = synchronized {
-    outstandingInstances += 1
+  def make() = {
+    latch.incr()
     val wrapped = new Service[Req, Rep] {
       def apply(request: Req) = service(request)
-      override def release() = SingletonFactory.this.synchronized {
-        outstandingInstances -= 1
-        if (outstandingInstances == 0 && needsRelease)
-          service.release()
-      }
+      override def release() = latch.decr()
     }
 
     Future.value(wrapped)
   }
 
-  def close() = synchronized {
-    if (outstandingInstances == 0)
-      service.release()
-    else
-      needsRelease = true
-  }
+  def close() = latch.await { service.release() }
 }

From ac38ec93b212d83f926347e90f6dcc270c3f884b Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Wed, 2 Feb 2011 11:19:15 -0800
Subject: [PATCH 08/43] style, -whitespacegasm

---
 .../com/twitter/finagle/builder/ClientBuilder.scala    |  3 +--
 .../com/twitter/finagle/service/SingletonFactory.scala |  6 ++----
 .../scala/com/twitter/finagle/util/FutureLatch.scala   | 10 ----------
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index b7c6ca0bb1..f4978a5160 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -295,8 +295,7 @@ case class ClientBuilder[Req, Rep](
       factory
     }
 
-    new LoadBalancedFactory(
-      hostFactories, new LeastQueuedStrategy[Req, Rep])
+    new LoadBalancedFactory(hostFactories, new LeastQueuedStrategy[Req, Rep])
   }
 
   def build(): Service[Req, Rep] = {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
index 734c5f6a55..8838392e51 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
@@ -10,14 +10,12 @@ class SingletonFactory[Req, Rep](service: Service[Req, Rep])
 {
   private[this] var latch = new FutureLatch
 
-  def make() = {
+  def make() = Future {
     latch.incr()
-    val wrapped = new Service[Req, Rep] {
+    new Service[Req, Rep] {
       def apply(request: Req) = service(request)
       override def release() = latch.decr()
     }
-
-    Future.value(wrapped)
   }
 
   def close() = latch.await { service.release() }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
index 4db18efaf4..4a07f33ba9 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
@@ -40,13 +40,3 @@ class FutureLatch(initialCount: Int = 0) {
     }
   }
 }
-
-
-
-
-
-
-
-
-
-

From d04e3eddd56a2f2209218a3b5a58db9c56d3026e Mon Sep 17 00:00:00 2001
From: Nick Kallen <nkallen@tallbro.local>
Date: Wed, 2 Feb 2011 11:41:39 -0800
Subject: [PATCH 09/43] dispose on channel cloes

---
 .../scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala b/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
index cf717a5775..8372ab99a1 100644
--- a/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
+++ b/finagle-stream/src/main/scala/com/twitter/finagle/stream/ChannelToHttpChunk.scala
@@ -36,6 +36,7 @@ class ChannelToHttpChunk extends SimpleChannelDownstreamHandler {
       channel.closes.respond { _ =>
         val ignoreOutcome = new DefaultChannelFuture(ctx.getChannel, false)
         channelRef.set(null)
+        observerRef.get.dispose()
         observerRef.set(null)
         Channels.write(ctx, ignoreOutcome, new DefaultHttpChunkTrailer)
       }

From 8e10521f388da080ae26a809febf8de525fad02a Mon Sep 17 00:00:00 2001
From: Nick Kallen <nkallen@tallbro.local>
Date: Wed, 2 Feb 2011 14:28:28 -0800
Subject: [PATCH 10/43] spike on piping hosebird to kestrel

---
 .../twitter/finagle/kestrel/java/Client.java  | 16 +++-
 .../finagle/kestrel/java/ClientBase.java      |  9 ++-
 .../com/twitter/finagle/kestrel/Client.scala  | 80 +++++++++++++------
 .../kestrel/integration/ClientSpec.scala      | 15 ++--
 .../finagle/stream/HttpChunkToChannel.scala   |  8 +-
 .../finagle/stream/Stream2Kestrel.scala       | 47 +++++++++++
 project/build/Project.scala                   |  2 +-
 7 files changed, 137 insertions(+), 40 deletions(-)
 create mode 100644 finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala

diff --git a/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/Client.java b/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/Client.java
index 3032e779b7..f89a3bbedb 100644
--- a/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/Client.java
+++ b/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/Client.java
@@ -1,6 +1,7 @@
 package com.twitter.finagle.kestrel.java;
 
 import com.twitter.concurrent.Channel;
+import com.twitter.concurrent.ChannelSource;
 import com.twitter.finagle.Service;
 import com.twitter.finagle.kestrel.protocol.Command;
 import com.twitter.finagle.kestrel.protocol.Response;
@@ -61,9 +62,16 @@ public static Client newInstance(Service<Command, Response> finagleClient) {
    * A friendly Channel object for Dequeueing items from a queue as they arrive.
    * @param key queue name
    * @param waitFor if the queue is empty, wait up to this duration for something to arrive before explicitly calling dequeueing again. A sensible value for this is infinity.
-   * @return
+   * @return a ChannelSource<ChannelBuffer>
+   */
+  abstract public Channel<ChannelBuffer> sink(String key, Duration waitFor);
+
+  /**
+   * A friendly ChannelSource object for Enqueuing items from a queue as they arrive.
+   * @param key queue name
+   * @return a ChannelSource<ChannelBuffer>
    */
-  abstract public Channel channel(String key, Duration waitFor);
+  abstract public ChannelSource<ChannelBuffer> source(String key);
 
   /**
    * Dequeue an item
@@ -81,8 +89,8 @@ public Future<ChannelBuffer> get(String key) {
    * @param key the queue name
    * @return
    */
-  public Channel channel(String key) {
-    return this.channel(key, Duration.apply(10, TimeUnit.SECONDS));
+  public Channel<ChannelBuffer> sink(String key) {
+    return this.sink(key, Duration.apply(10, TimeUnit.SECONDS));
   }
 
   /**
diff --git a/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/ClientBase.java b/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/ClientBase.java
index 96318dfb06..083b501108 100644
--- a/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/ClientBase.java
+++ b/finagle-kestrel/src/main/java/com/twitter/finagle/kestrel/java/ClientBase.java
@@ -1,6 +1,7 @@
 package com.twitter.finagle.kestrel.java;
 
 import com.twitter.concurrent.Channel;
+import com.twitter.concurrent.ChannelSource;
 import com.twitter.finagle.kestrel.protocol.Response;
 import com.twitter.util.Duration;
 import com.twitter.util.Function;
@@ -41,7 +42,11 @@ public Future<Response> flush(String key) {
     return underlying.delete(key);
   }
 
-  public Channel channel(String key, Duration waitFor) {
-    return underlying.channel(key, waitFor);
+  public Channel<ChannelBuffer> sink(String key, Duration waitFor) {
+    return underlying.sink(key, waitFor);
+  }
+
+  public ChannelSource<ChannelBuffer> source(String key) {
+    return underlying.source(key);
   }
 }
\ No newline at end of file
diff --git a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
index 405a257860..f6f79d02b9 100644
--- a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
+++ b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
@@ -6,23 +6,64 @@ import com.twitter.conversions.time._
 import com.twitter.finagle.Service
 import com.twitter.finagle.kestrel.protocol._
 import com.twitter.finagle.memcached.util.ChannelBufferUtils._
-import com.twitter.concurrent.{Channel, Topic}
+import com.twitter.concurrent.{ChannelSource, Channel}
+import com.twitter.finagle.builder.ClientBuilder
 
 object Client {
   def apply(raw: Service[Command, Response]): Client = {
     new ConnectedClient(raw)
   }
+
+  def apply(hosts: String): Client = {
+    val service = ClientBuilder()
+      .codec(new Kestrel)
+      .hosts(hosts)
+      .build()
+    apply(service)
+  }
 }
 
 /**
  * A friendly Kestrel client Interface.
  */
 trait Client {
+  /**
+   * Enqueue an item.
+   *
+   * @param  expiry  how long the item is valid for (Kestrel will delete the item if it isn't dequeued in time.
+   */
   def set(queueName: String, value: ChannelBuffer, expiry: Time = Time.epoch): Future[Response]
+
+  /**
+   * Dequeue an item.
+   *
+   * @param  waitUpTo  if the queue is empty, indicate to the Kestrel server how long to block the operation, waiting for something to arrive, before returning None
+   */
   def get(queueName: String, waitUpTo: Duration = 0.seconds): Future[Option[ChannelBuffer]]
+
+  /**
+   * Delete a queue. Removes the journal file on the remote server.
+   */
   def delete(queueName: String): Future[Response]
+
+  /**
+   * Flush a queue. Empties all items from the queue without deleting the journal.
+   */
   def flush(queueName: String): Future[Response]
-  def channel(queueName: String, waitUpTo: Duration = 0.seconds): Channel[ChannelBuffer]
+
+  /**
+   * Get a channel for the given queue
+   *
+   * @return A Channel object that you can receive items from as they arrive.
+   */
+  def sink(queueName: String, waitUpTo: Duration = 0.seconds): Channel[ChannelBuffer]
+
+  /**
+   * Get a ChannelSource for the given queue
+   *
+   * @return  A ChannelSource that you can send items to.
+   */
+  def source(queueName: String): ChannelSource[ChannelBuffer]
 }
 
 /**
@@ -39,20 +80,10 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
     underlying(Delete(queueName))
   }
 
-  /**
-   * Enqueue an item.
-   *
-   * @param  expiry  how long the item is valid for (Kestrel will delete the item if it isn't dequeued in time.
-   */
   def set(queueName: String, value: ChannelBuffer, expiry: Time = Time.epoch) = {
     underlying(Set(queueName, expiry, value))
   }
 
-  /**
-   * Dequeue an item.
-   *
-   * @param  waitUpTo  if the queue is empty, indicate to the Kestrel server how long to block the operation, waiting for something to arrive, before returning None
-   */
   def get(queueName: String, waitUpTo: Duration = 0.seconds) = {
     underlying(Get(queueName, collection.Set(Timeout(waitUpTo)))) map {
       case Values(Seq()) => None
@@ -60,20 +91,23 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
     }
   }
 
-  /**
-   * Get a channel for the given queue
-   *
-   * @return  A Channel object that you can receive items from as they arrive.
-   */
-  def channel(queueName: String, waitUpTo: Duration = 10.seconds): Channel[ChannelBuffer] = {
-    val channel = new Topic[ChannelBuffer]
-    channel.onReceive {
-      receive(queueName, channel, waitUpTo, collection.Set(Open()))
+  def sink(queueName: String, waitUpTo: Duration = 10.seconds): Channel[ChannelBuffer] = {
+    val sink = new ChannelSource[ChannelBuffer]
+    sink.responds.first.foreach { _ =>
+      receive(queueName, sink, waitUpTo, collection.Set(Open()))
+    }
+    sink
+  }
+
+  def source(queueName: String): ChannelSource[ChannelBuffer] = {
+    val source = new ChannelSource[ChannelBuffer]
+    source.respond(source) { item =>
+      set(queueName, item)
     }
-    channel
+    source
   }
 
-  private[this] def receive(queueName: String, channel: Topic[ChannelBuffer], waitUpTo: Duration, options: collection.Set[GetOption]) {
+  private[this] def receive(queueName: String, channel: ChannelSource[ChannelBuffer], waitUpTo: Duration, options: collection.Set[GetOption]) {
     if (channel.isOpen) {
       underlying(Get(queueName, collection.Set(Timeout(waitUpTo)) ++ options)) respond {
         case Return(Values(Seq(Value(key, item)))) =>
diff --git a/finagle-kestrel/src/test/scala/com/twitter/finagle/kestrel/integration/ClientSpec.scala b/finagle-kestrel/src/test/scala/com/twitter/finagle/kestrel/integration/ClientSpec.scala
index a129e6019b..e997b22c95 100644
--- a/finagle-kestrel/src/test/scala/com/twitter/finagle/kestrel/integration/ClientSpec.scala
+++ b/finagle-kestrel/src/test/scala/com/twitter/finagle/kestrel/integration/ClientSpec.scala
@@ -8,7 +8,6 @@ import org.jboss.netty.util.CharsetUtil
 import com.twitter.finagle.memcached.util.ChannelBufferUtils._
 import collection.mutable.ListBuffer
 import com.twitter.util.CountDownLatch
-import com.twitter.concurrent.Value
 import com.twitter.conversions.time._
 import org.jboss.netty.buffer.ChannelBuffer
 
@@ -39,9 +38,9 @@ object ClientSpec extends Specification {
           client.set("foo", "baz")()
           client.set("foo", "boing")()
 
-          val channel = client.channel("foo")
+          val channel = client.sink("foo")
           val latch = new CountDownLatch(3)
-          channel.foreach {
+          channel.respond(this) {
             case item =>
               result += item.toString(CharsetUtil.UTF_8)
               latch.countDown()
@@ -55,13 +54,13 @@ object ClientSpec extends Specification {
           client.set("foo", "bar")()
 
           var result: ChannelBuffer = null
-          var channel = client.channel("foo")
+          var channel = client.sink("foo")
           val latch = new CountDownLatch(1)
-          channel.foreach {
-            case item => throw new Exception
+          channel.respond(this) { item =>
+            throw new Exception
           }
-          channel = client.channel("foo")
-          channel.foreach {
+          channel = client.sink("foo")
+          channel.respond(this) {
             case item =>
               result = item
               latch.countDown()
diff --git a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
index f9fa5917ce..54798c26e1 100644
--- a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
+++ b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
@@ -1,10 +1,10 @@
 package com.twitter.finagle.stream
 
-import org.jboss.netty.handler.codec.http.{HttpChunkTrailer, HttpChunk, HttpMessage}
 import org.jboss.netty.buffer.ChannelBuffer
 import org.jboss.netty.channel.{Channels, MessageEvent, ChannelHandlerContext, SimpleChannelUpstreamHandler}
 import java.util.concurrent.atomic.{AtomicInteger, AtomicReference}
 import com.twitter.concurrent.{Serialized, ChannelSource}
+import org.jboss.netty.handler.codec.http._
 
 /**
  * Client handler for a streaming protocol.
@@ -14,10 +14,14 @@ class HttpChunkToChannel extends SimpleChannelUpstreamHandler with Serialized {
     new AtomicReference[com.twitter.concurrent.ChannelSource[ChannelBuffer]](null)
 
   override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) = e.getMessage match {
-    case message: HttpMessage =>
+    case message: HttpResponse =>
+      println(message)
+      require(message.getStatus == HttpResponseStatus.OK,
+        "Error: " + message.getStatus)
       val source = new ChannelSource[ChannelBuffer]
       require(channelRef.compareAndSet(null, source),
         "Channel is already busy")
+
       ctx.getChannel.setReadable(false)
       source.responds.first.respond { _ =>
         if (!message.isChunked) {
diff --git a/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala b/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala
new file mode 100644
index 0000000000..cd4a4a365f
--- /dev/null
+++ b/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala
@@ -0,0 +1,47 @@
+package com.twitter.finagle.stream
+
+import org.specs.Specification
+import com.twitter.finagle.builder.ClientBuilder
+import org.jboss.netty.util.CharsetUtil
+import com.twitter.finagle.kestrel.{Client => Kestrel}
+import com.twitter.finagle.Service
+import org.jboss.netty.handler.codec.http._
+import com.twitter.concurrent.Channel
+import org.jboss.netty.buffer.ChannelBuffer
+import java.net.URI
+import com.twitter.util.Future
+
+object Hose {
+  def apply(uriString: String, headers: (String, String)*): Channel[ChannelBuffer] = {
+    val uri = new URI(uriString)
+    val request = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/1/statuses/sample.json")
+    headers foreach { case (key, value) =>
+      request.addHeader(key, value)
+    }
+    request.addHeader("Host", uri.getHost)
+    val port = if (uri.getPort < 0) 80 else uri.getPort
+    val service = ClientBuilder()
+      .codec(new Stream)
+      .hosts(uri.getHost + ":" + port)
+      .build()
+    service(request)()
+  }
+}
+
+object Stream2Kestrel extends Specification {
+  "Stream Piped to Kestrel" should {
+    "make you wet your pants" in {
+      val stream = ClientBuilder()
+        .codec(new Stream)
+        .hosts("stream.twitter.com:80")
+        .build()
+      val hose = Hose(
+        "http://stream.twitter.com/1/statuses/sample.json",
+        "Authorization" -> "Basic cGl2b3Q0OnBpdm90czJwYW5kYXM=")
+      val kestrel = Kestrel("localhost:22133")
+
+//      hose.pipe(kestrel.source("tweets"))
+//      kestrel.sink("tweets").pipe(kestrel.source("tweets2"))
+    }
+  }
+}
diff --git a/project/build/Project.scala b/project/build/Project.scala
index 8e382f4f87..874945de51 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -53,7 +53,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
    */
   val streamProject = project(
     "finagle-stream", "finagle-stream",
-    new StreamProject(_), coreProject)
+    new StreamProject(_), coreProject, kestrelProject)
 
   /**
    * finagle-stress has stress/integration test suites & tools for

From 32a14f46aa287dcd3eb048f5585612e2ac836463 Mon Sep 17 00:00:00 2001
From: Nick Kallen <nkallen@tallbro.local>
Date: Wed, 2 Feb 2011 14:30:36 -0800
Subject: [PATCH 11/43] remove println

---
 .../scala/com/twitter/finagle/stream/HttpChunkToChannel.scala    | 1 -
 1 file changed, 1 deletion(-)

diff --git a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
index 54798c26e1..9b3e2c5b0f 100644
--- a/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
+++ b/finagle-stream/src/main/scala/com/twitter/finagle/stream/HttpChunkToChannel.scala
@@ -15,7 +15,6 @@ class HttpChunkToChannel extends SimpleChannelUpstreamHandler with Serialized {
 
   override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) = e.getMessage match {
     case message: HttpResponse =>
-      println(message)
       require(message.getStatus == HttpResponseStatus.OK,
         "Error: " + message.getStatus)
       val source = new ChannelSource[ChannelBuffer]

From b895b2e270fa61fef156e49bee12eb2dfa4618fd Mon Sep 17 00:00:00 2001
From: Nick Kallen <nkallen@tallbro.local>
Date: Wed, 2 Feb 2011 15:59:51 -0800
Subject: [PATCH 12/43] pipe kestrel2kestrel

---
 .../scala/com/twitter/finagle/kestrel/Client.scala   |  7 +++++++
 .../com/twitter/finagle/kestrel/protocol/Show.scala  |  2 +-
 .../memcached/protocol/text/AbstractDecoder.scala    | 12 +++++++++++-
 .../com/twitter/finagle/stream/Stream2Kestrel.scala  |  5 +++--
 project/build/Project.scala                          |  2 +-
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
index f6f79d02b9..b4987f9d33 100644
--- a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
+++ b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/Client.scala
@@ -94,6 +94,7 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
   def sink(queueName: String, waitUpTo: Duration = 10.seconds): Channel[ChannelBuffer] = {
     val sink = new ChannelSource[ChannelBuffer]
     sink.responds.first.foreach { _ =>
+      println("a responder registered...")
       receive(queueName, sink, waitUpTo, collection.Set(Open()))
     }
     sink
@@ -102,6 +103,7 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
   def source(queueName: String): ChannelSource[ChannelBuffer] = {
     val source = new ChannelSource[ChannelBuffer]
     source.respond(source) { item =>
+      println("putting item==")
       set(queueName, item)
     }
     source
@@ -109,8 +111,10 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
 
   private[this] def receive(queueName: String, channel: ChannelSource[ChannelBuffer], waitUpTo: Duration, options: collection.Set[GetOption]) {
     if (channel.isOpen) {
+      println("calling GET")
       underlying(Get(queueName, collection.Set(Timeout(waitUpTo)) ++ options)) respond {
         case Return(Values(Seq(Value(key, item)))) =>
+          println("getsting an item===")
           try {
             channel.send(item)
             receive(queueName, channel, waitUpTo, collection.Set(Close(), Open()))
@@ -120,8 +124,11 @@ protected class ConnectedClient(underlying: Service[Command, Response]) extends
               channel.close()
           }
         case Return(Values(Seq())) =>
+          println("getting item==")
           receive(queueName, channel, waitUpTo, collection.Set(Open()))
         case Throw(e) =>
+          e.printStackTrace()
+          println("error")
           channel.close()
       }
     }
diff --git a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/protocol/Show.scala b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/protocol/Show.scala
index ec11a0eb34..150223a24a 100644
--- a/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/protocol/Show.scala
+++ b/finagle-kestrel/src/main/scala/com/twitter/finagle/kestrel/protocol/Show.scala
@@ -54,7 +54,7 @@ class CommandToEncoding extends OneToOneEncoder {
         var key = queueName.toString(CharsetUtil.US_ASCII)
         options foreach { option =>
           val optionString = option match {
-            case Timeout(timeout) => "t=" + timeout.inSeconds
+            case Timeout(timeout) => "t=" + timeout.inMilliseconds.toString
             case Open() => OPEN
             case Close() => CLOSE
             case Abort() => ABORT
diff --git a/finagle-memcached/src/main/scala/com/twitter/finagle/memcached/protocol/text/AbstractDecoder.scala b/finagle-memcached/src/main/scala/com/twitter/finagle/memcached/protocol/text/AbstractDecoder.scala
index 44932a2422..f8d1a6f08c 100644
--- a/finagle-memcached/src/main/scala/com/twitter/finagle/memcached/protocol/text/AbstractDecoder.scala
+++ b/finagle-memcached/src/main/scala/com/twitter/finagle/memcached/protocol/text/AbstractDecoder.scala
@@ -7,10 +7,20 @@ import org.jboss.netty.channel._
 import collection.mutable.ArrayBuffer
 import com.twitter.finagle.memcached.util.ParserUtils
 import com.twitter.finagle.memcached.util.ChannelBufferUtils._
+import org.jboss.netty.util.CharsetUtil
 
 object AbstractDecoder {
   private val DELIMETER = ChannelBuffers.wrappedBuffer("\r\n".getBytes)
   private val SKIP_SPACE = 1
+  private val FIND_CRLF = new ChannelBufferIndexFinder() {
+    def find(buffer: ChannelBuffer, guessedIndex: Int): Boolean = {
+      if (buffer.writerIndex < guessedIndex + DELIMETER.readableBytes - 1) return false
+
+      val cr = buffer.getByte(guessedIndex)
+      val lf = buffer.getByte(guessedIndex + 1)
+      cr == '\r' && lf == '\n'
+    }
+}
 }
 
 abstract class AbstractDecoder extends FrameDecoder {
@@ -29,7 +39,7 @@ abstract class AbstractDecoder extends FrameDecoder {
   }
 
   protected def decodeLine(buffer: ChannelBuffer, needsData: Seq[ChannelBuffer] => Option[Int])(continue: Seq[ChannelBuffer] => Decoding): Decoding = {
-    val frameLength = buffer.bytesBefore(ChannelBufferIndexFinder.CRLF)
+    val frameLength = buffer.bytesBefore(FIND_CRLF)
     if (frameLength < 0) {
       needMoreData
     } else {
diff --git a/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala b/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala
index cd4a4a365f..ce1be172ce 100644
--- a/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala
+++ b/finagle-stream/src/test/scala/com/twitter/finagle/stream/Stream2Kestrel.scala
@@ -31,6 +31,7 @@ object Hose {
 object Stream2Kestrel extends Specification {
   "Stream Piped to Kestrel" should {
     "make you wet your pants" in {
+      skip("Not yet working")
       val stream = ClientBuilder()
         .codec(new Stream)
         .hosts("stream.twitter.com:80")
@@ -40,8 +41,8 @@ object Stream2Kestrel extends Specification {
         "Authorization" -> "Basic cGl2b3Q0OnBpdm90czJwYW5kYXM=")
       val kestrel = Kestrel("localhost:22133")
 
-//      hose.pipe(kestrel.source("tweets"))
-//      kestrel.sink("tweets").pipe(kestrel.source("tweets2"))
+      hose.pipe(kestrel.source("tweets"))
+      kestrel.sink("tweets").pipe(kestrel.source("tweets2"))
     }
   }
 }
diff --git a/project/build/Project.scala b/project/build/Project.scala
index 874945de51..9a9134ecdf 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -71,7 +71,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
     val nettyRepo =
       "repository.jboss.org" at "http://repository.jboss.org/nexus/content/groups/public/"
     val netty     = "org.jboss.netty"      %  "netty"     % "3.2.3.Final"
-    val util      = "com.twitter"          %  "util"      % "1.5.2"
+    val util      = "com.twitter"          %  "util"      % "1.5.3"
 
     val mockito   = "org.mockito"             % "mockito-all" % "1.8.5" % "test" withSources()
     val specs     = "org.scala-tools.testing" % "specs_2.8.0" % "1.6.5" % "test" withSources()

From 4ae82b99c9dd7ca685b28261dbef837de3e21a15 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Wed, 2 Feb 2011 18:06:56 -0800
Subject: [PATCH 13/43] - make filter composition responsible for dealing with
 premature service releases.  this simplifies pool implementations and
 enhances general composability.

- implement an ExpiringService that expires a wrapped service after a
certain idle time.
---
 .../scala/com/twitter/finagle/Service.scala   |  13 ++-
 .../twitter/finagle/pool/CachingPool.scala    |   8 +-
 .../twitter/finagle/pool/WatermarkPool.scala  |   8 +-
 .../finagle/service/ExpiringService.scala     |  57 ++++++++++
 .../RefcountedService.scala}                  |   8 +-
 .../twitter/finagle/util/FutureLatch.scala    |   3 +-
 .../scala/com/twitter/finagle/Timer.scala     |  38 +++++++
 .../finagle/pool/CachingPoolSpec.scala        | 103 ++++++------------
 .../finagle/pool/PoolServiceWrapperSpec.scala |  44 --------
 .../finagle/service/ExpiringServiceSpec.scala |  71 ++++++++++++
 .../service/RefcountedServiceSpec.scala       |  38 +++++++
 11 files changed, 265 insertions(+), 126 deletions(-)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
 rename finagle-core/src/main/scala/com/twitter/finagle/{pool/PoolServiceWrapper.scala => service/RefcountedService.scala} (62%)
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/Timer.scala
 delete mode 100644 finagle-core/src/test/scala/com/twitter/finagle/pool/PoolServiceWrapperSpec.scala
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/service/ExpiringServiceSpec.scala
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/service/RefcountedServiceSpec.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/Service.scala b/finagle-core/src/main/scala/com/twitter/finagle/Service.scala
index d0c7c6f840..7f25197562 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/Service.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/Service.scala
@@ -2,6 +2,8 @@ package com.twitter.finagle
 
 import com.twitter.util.Future
 
+import com.twitter.finagle.service.RefcountedService
+
 /**
  * A Service is an asynchronous function from Request to Future[Response]. It is the
  * basic unit of an RPC interface.
@@ -21,7 +23,8 @@ abstract class Service[-Req, +Rep] extends (Req => Future[Rep]) {
   def apply(request: Req): Future[Rep]
 
   /**
-   * Relinquishes the use of this service instance.
+   * Relinquishes the use of this service instance. Behavior is
+   * undefined is apply() is called after resources are relinquished.
    */
   def release() = ()
 
@@ -118,9 +121,11 @@ abstract class Filter[-ReqIn, +RepOut, +ReqOut, -RepIn]
    *
    */
   def andThen(service: Service[ReqOut, RepIn]) = new Service[ReqIn, RepOut] {
-    def apply(request: ReqIn) = Filter.this.apply(request, service)
-    override def release() = service.release()
-    override def isAvailable = service.isAvailable
+    private[this] val refcounted = new RefcountedService(service)
+
+    def apply(request: ReqIn) = Filter.this.apply(request, refcounted)
+    override def release() = refcounted.release()
+    override def isAvailable = refcounted.isAvailable
   }
 
   def andThen(factory: ServiceFactory[ReqOut, RepIn]): ServiceFactory[ReqIn, RepOut] =
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/pool/CachingPool.scala b/finagle-core/src/main/scala/com/twitter/finagle/pool/CachingPool.scala
index 275cc6a596..9e6555ac50 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/pool/CachingPool.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/pool/CachingPool.scala
@@ -22,9 +22,11 @@ class CachingPool[Req, Rep](
   private[this] var isScheduled = false
 
   private[this] class WrappedService(underlying: Service[Req, Rep])
-    extends PoolServiceWrapper[Req, Rep](underlying)
+    extends Service[Req, Rep]
   {
-    def doRelease() = CachingPool.this.synchronized {
+    def apply(request: Req) = underlying.apply(request)
+
+    override def release() = CachingPool.this.synchronized {
       if (underlying.isAvailable && isOpen) {
         deathRow += ((Time.now, underlying))
         if (!isScheduled) {
@@ -35,6 +37,8 @@ class CachingPool[Req, Rep](
         underlying.release()
       }
     }
+
+    override def isAvailable = underlying.isAvailable
   }
 
   private[this] def collect(): Unit = synchronized {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/pool/WatermarkPool.scala b/finagle-core/src/main/scala/com/twitter/finagle/pool/WatermarkPool.scala
index 885607e788..046b7f1ab7 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/pool/WatermarkPool.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/pool/WatermarkPool.scala
@@ -30,9 +30,11 @@ class WatermarkPool[Req, Rep](
   private[this] var isOpen      = true
 
   private[this] class ServiceWrapper(underlying: Service[Req, Rep])
-    extends PoolServiceWrapper[Req, Rep](underlying)
+    extends Service[Req, Rep]
   {
-    override def doRelease() = WatermarkPool.this.synchronized {
+    def apply(request: Req) = underlying(request)
+    
+    override def release() = WatermarkPool.this.synchronized {
       if (!isOpen) {
         underlying.release()
         numServices -= 1
@@ -56,6 +58,8 @@ class WatermarkPool[Req, Rep](
         numServices -= 1
       }
     }
+
+    override def isAvailable = underlying.isAvailable
   }
 
   @tailrec private[this] def dequeue(): Option[Service[Req, Rep]] = {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
new file mode 100644
index 0000000000..5b9868e351
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
@@ -0,0 +1,57 @@
+package com.twitter.finagle.service
+
+import com.twitter.util
+import com.twitter.util.{Duration, Future}
+
+import com.twitter.finagle.util.Timer
+import com.twitter.finagle.{Service, WriteException, ChannelClosedException}
+
+class ExpiringService[Req, Rep](
+    underlying: Service[Req, Rep],
+    maxIdleTime: Duration,
+    timer: util.Timer = Timer.default)
+  extends Service[Req, Rep]
+{
+  private[this] var requestCount = 0
+  private[this] var expired = false
+  private[this] var task: Option[com.twitter.util.TimerTask] =
+    Some(timer.schedule(maxIdleTime.fromNow) { doExpire() })
+
+  private[this] def doExpire() = synchronized {
+    // We check requestCount == 0 here to avoid the race between
+    // cancellation & running of the timer.
+    if (!expired && requestCount == 0) {
+      underlying.release()
+      expired = true
+    }
+  }
+
+  def apply(request: Req): Future[Rep] = synchronized {
+    if (expired) {
+      return Future.exception(
+        new WriteException(new ChannelClosedException))
+    }
+
+    requestCount += 1
+    if (requestCount == 1) {
+      // Cancel the existing timer.
+      task foreach { _.cancel() }
+      task = None
+    }
+
+    underlying(request) ensure {
+      synchronized {
+        requestCount -= 1
+        if (requestCount == 0) {
+          require(!task.isDefined)
+          task = Some(timer.schedule(maxIdleTime.fromNow) { doExpire() })
+        }
+      }
+    }
+  }
+
+  override def isAvailable: Boolean = {
+    synchronized { if (expired) return false }
+    underlying.isAvailable
+  }
+}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/pool/PoolServiceWrapper.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/RefcountedService.scala
similarity index 62%
rename from finagle-core/src/main/scala/com/twitter/finagle/pool/PoolServiceWrapper.scala
rename to finagle-core/src/main/scala/com/twitter/finagle/service/RefcountedService.scala
index 3358895002..8d6e270b28 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/pool/PoolServiceWrapper.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/RefcountedService.scala
@@ -1,12 +1,12 @@
-package com.twitter.finagle.pool
+package com.twitter.finagle.service
 
 import com.twitter.finagle.Service
 import com.twitter.finagle.util.FutureLatch
 
-private[pool] abstract class PoolServiceWrapper[Req, Rep](underlying: Service[Req, Rep])
+class RefcountedService[Req, Rep](underlying: Service[Req, Rep])
   extends Service[Req, Rep]
 {
-  private[this] val replyLatch = new FutureLatch
+  protected[this] val replyLatch = new FutureLatch
 
   def apply(request: Req) = {
     replyLatch.incr()
@@ -16,5 +16,5 @@ private[pool] abstract class PoolServiceWrapper[Req, Rep](underlying: Service[Re
   override def isAvailable = underlying.isAvailable
   override final def release() = replyLatch.await { doRelease() }
 
-  protected def doRelease()
+  protected def doRelease() = underlying.release()
 }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
index 4a07f33ba9..04dec91008 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/FutureLatch.scala
@@ -25,7 +25,7 @@ class FutureLatch(initialCount: Int = 0) {
   /**
    * Increment the latch.
    */
-  def incr() = synchronized { count += 1 }
+  def incr() = synchronized { count += 1; count }
 
   /**
    * Decrement the latch. If the latch value reaches 0, awaiting
@@ -38,5 +38,6 @@ class FutureLatch(initialCount: Int = 0) {
       waiters foreach { _() }
       waiters.clear()
     }
+    count
   }
 }
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/Timer.scala b/finagle-core/src/test/scala/com/twitter/finagle/Timer.scala
new file mode 100644
index 0000000000..e191dea1b4
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/Timer.scala
@@ -0,0 +1,38 @@
+package com.twitter.finagle
+
+import collection.mutable.ArrayBuffer
+
+import com.twitter.util.{Time, Duration}
+
+class MockTimer extends com.twitter.util.Timer {
+  case class Task(when: Time, runner: Function0[Unit])
+    extends com.twitter.util.TimerTask
+  {
+    var isCancelled = false
+    def cancel() { isCancelled = true }
+  }
+
+  var isStopped = false
+  var tasks = ArrayBuffer[Task]()
+
+  def tick() {
+    if (isStopped)
+      throw new Exception("timer is stopped already")
+
+    val now = Time.now
+    val (toRun, toQueue) = tasks.partition { task => task.when <= now }
+    tasks = toQueue
+    toRun filter { !_.isCancelled } foreach { _.runner() }
+  }
+
+  def schedule(when: Time)(f: => Unit) = {
+    val task = Task(when, () => f)
+    tasks += task
+    task
+  }
+
+  def schedule(when: Time, period: Duration)(f: => Unit) =
+    throw new Exception("periodic scheduling not supported")
+
+  def stop() { isStopped = true }
+}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/pool/CachingPoolSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/pool/CachingPoolSpec.scala
index 43df0f934a..cd70706eb5 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/pool/CachingPoolSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/pool/CachingPoolSpec.scala
@@ -11,22 +11,11 @@ import com.twitter.util.{Time, Duration, Future}
 import com.twitter.conversions.time._
 
 import com.twitter.finagle.{Service, ServiceFactory}
+import com.twitter.finagle.MockTimer
 
 object CachingPoolSpec extends Specification with Mockito {
   "CachingPool" should {
-    val timer = new util.Timer {
-      val scheduled = Queue[(Time, Function0[Unit])]()
-      def schedule(when: Time)(f: => Unit) = {
-        scheduled += (when, () => f)
-        null  // not used by caller in this case.
-      }
- 
-      def schedule(when: Time, period: Duration)(f: => Unit) =
-        throw new Exception("periodic scheduling not supported")
- 
-      def stop() = ()
-    }
- 
+    val timer = new MockTimer
     val obj = mock[Object]
     val underlying = mock[ServiceFactory[Any, Any]]
     val underlyingService = mock[Service[Any, Any]]
@@ -41,21 +30,20 @@ object CachingPoolSpec extends Specification with Mockito {
         val f = cachingPool.make()()
         f(123)() must be_==(obj)
         there was one(underlying).make()
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
 
         f.release()
         there was one(underlyingService).isAvailable
         there was no(underlyingService).release()
-        timer.scheduled must haveSize(1)
-        val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-        timeToSchedule must be_==(Time.now + 5.seconds)
+        timer.tasks must haveSize(1)
+        timer.tasks.head.when must be_==(Time.now + 5.seconds)
  
         // Reap!
         timeControl.advance(5.seconds)
-        funToSchedule()
+        timer.tick()
         there was one(underlyingService).release()
  
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
       }
     }
  
@@ -66,33 +54,26 @@ object CachingPoolSpec extends Specification with Mockito {
 
         there was one(underlying).make()
         there was no(underlyingService).release()
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
 
         timeControl.advance(4.seconds)
 
         cachingPool.make()().release()
         there was one(underlying).make()
         there was no(underlyingService).release()
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
  
         // Originally scheduled time.
         timeControl.advance(1.second)
+        timer.tick()
  
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now)
-          funToSchedule()
-        }
- 
-        timer.scheduled must haveSize(1)  // reschedule
+        timer.tasks must haveSize(1)  // reschedule
         there was no(underlyingService).release()
  
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now + 4.seconds)
-          timeControl.advance(5.seconds)
-          funToSchedule()
-        }
+        timer.tasks.head.when must be_==(Time.now + 4.seconds)
+        timeControl.advance(5.seconds)
+        timer.tick()
+        timer.tasks must beEmpty
  
         there was one(underlyingService).release()
       }
@@ -133,43 +114,31 @@ object CachingPoolSpec extends Specification with Mockito {
           timeControl.advance(1.second)
         }
      
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
         ss foreach { s => there was no(s).release() }
         timeControl.advance(2.seconds)
      
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now)
-          funToSchedule()
-        }
- 
+        timer.tick()
+
         there was one(s0).release()
  
         timeControl.advance(1.second)
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
  
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now)
-          funToSchedule()
-        }
+        timer.tick()
  
-        timer.scheduled must haveSize(1)
-        timer.scheduled.head._1 must be_==(Time.now + 1.second)
+        timer.tasks must haveSize(1)
+        timer.tasks.head.when must be_==(Time.now + 1.second)
  
         // Take it!
         cachingPool.make()()(123)() must be_==(o2)
 
         timeControl.advance(1.second)
  
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now)
-          funToSchedule()
-        }
+        timer.tick()
  
         // Nothing left.
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
       }
     }
  
@@ -182,37 +151,33 @@ object CachingPoolSpec extends Specification with Mockito {
         val cachingPool = new CachingPool[Any, Any](underlying, 5.seconds, timer)
         underlying.make() returns Future.value(underlyingService)
 
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
         val service = cachingPool.make()()
         service(123)() must be_==(obj)
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
 
         service.release()
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
         there was no(underlyingService).release()
 
-        timer.scheduled.head._1 must be_==(Time.now + 5.seconds)
+        timer.tasks.head.when must be_==(Time.now + 5.seconds)
 
         timeControl.advance(1.second)
 
         cachingPool.make()()(123)() must be_==(obj)
 
         timeControl.advance(4.seconds)
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
 
-        {
-          val (timeToSchedule, funToSchedule) = timer.scheduled.dequeue()
-          timeToSchedule must be_==(Time.now)
-          funToSchedule()
-        }
- 
+        timer.tick()
+        
         there was no(underlyingService).release()
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
 
         service.release()
 
         there was no(underlyingService).release()
-        timer.scheduled must haveSize(1)
+        timer.tasks must haveSize(1)
       }
     }
  
@@ -232,7 +197,7 @@ object CachingPoolSpec extends Specification with Mockito {
         there was one(underlyingService).release()
 
         // No need to clean up an already disposed object. 
-        timer.scheduled must beEmpty
+        timer.tasks must beEmpty
       }
     }
 
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/pool/PoolServiceWrapperSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/pool/PoolServiceWrapperSpec.scala
deleted file mode 100644
index 7feec0e1b1..0000000000
--- a/finagle-core/src/test/scala/com/twitter/finagle/pool/PoolServiceWrapperSpec.scala
+++ /dev/null
@@ -1,44 +0,0 @@
-package com.twitter.finagle.pool
-
-import org.specs.Specification
-import org.specs.mock.Mockito
-import org.mockito.Matchers
-
-import com.twitter.util.{Promise, Return}
-import com.twitter.finagle.Service
-
-object PoolServiceWrapperSpec extends Specification with Mockito {
-  "PoolServiceWrapper" should {
-    class DelegatingWrapper[Req, Rep](service: Service[Req, Rep])
-      extends PoolServiceWrapper[Req, Rep](service)
-    {
-      def doRelease() { didRelease() }
-      def didRelease() {}
-    }
-
-    val service = mock[Service[Any, Any]]
-    val promise = new Promise[Any]
-    service(Matchers.any) returns promise
-    val wrapper = spy(new DelegatingWrapper[Any, Any](service))
-
-    "call doRelease immediately when no requests have been made" in {
-      there was no(wrapper).didRelease()
-      wrapper.release()
-      there was one(wrapper).didRelease()
-    }
-
-    "call doRelease after pending request finishes" in {
-      val f = wrapper(123)
-      f.isDefined must beFalse
-      there was one(service)(123)
-
-      wrapper.release()
-      there was no(wrapper).didRelease()
-
-      promise() = Return(123)
-      there was one(wrapper).didRelease()
-      f.isDefined must beTrue
-      f() must be_==(123)
-    }
-  }
-}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/ExpiringServiceSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/service/ExpiringServiceSpec.scala
new file mode 100644
index 0000000000..f106932a7b
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/service/ExpiringServiceSpec.scala
@@ -0,0 +1,71 @@
+package com.twitter.finagle.service
+
+import org.specs.Specification
+import org.specs.mock.Mockito
+
+import com.twitter.finagle.{Service, WriteException}
+import com.twitter.finagle.MockTimer
+
+import com.twitter.util.{Time, Promise, Return}
+import com.twitter.conversions.time._
+
+object ExpiringServiceSpec extends Specification with Mockito {
+  "ExpiringService" should {
+    Time.withCurrentTimeFrozen { timeControl =>
+      val timer = new MockTimer
+      val underlying = mock[Service[Any, Any]]
+      val promise = new Promise[Int]
+      underlying(123) returns promise
+      underlying.isAvailable returns true
+
+      val service = new ExpiringService[Any, Any](underlying, 10.seconds, timer)
+      timer.tasks must haveSize(1)
+
+      "expire after the given idle time" in {
+        // For some reason, this complains of different types:
+        //   timer.tasks.head.when must be_==(Time.now + 10.seconds)
+        service.isAvailable must beTrue
+       
+        timeControl.advance(10.seconds)
+        timer.tick()
+       
+        service.isAvailable must beFalse
+        there was one(underlying).release()
+       
+        timer.tasks must beEmpty
+      }
+
+      "cancel the timer when a request is issued" in {
+        service(123)
+        timer.tasks must haveSize(1)
+        timer.tasks.head.isCancelled must beTrue
+      }
+
+      "restart the timer when the request finished" in {
+        service(123)
+        timer.tasks must haveSize(1)
+        timer.tasks.head.isCancelled must beTrue
+       
+        timeControl.advance(10.seconds)
+        timer.tick()
+       
+        timer.tasks must beEmpty
+        promise() = Return(321)
+        timer.tasks must haveSize(1)
+       
+        there was no(underlying).release()
+        timeControl.advance(10.seconds)
+        timer.tick()
+       
+        there was one(underlying).release()
+      }
+
+      "throw an write exception if we attempt to use an expired service" in {
+        timeControl.advance(10.seconds)
+        timer.tick()
+
+        service(132)() must throwA[WriteException]
+      }
+    }
+  }
+}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/RefcountedServiceSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/service/RefcountedServiceSpec.scala
new file mode 100644
index 0000000000..1c513fed0f
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/service/RefcountedServiceSpec.scala
@@ -0,0 +1,38 @@
+package com.twitter.finagle.service
+
+import org.specs.Specification
+import org.specs.mock.Mockito
+import org.mockito.Matchers
+
+import com.twitter.util.{Promise, Return}
+import com.twitter.finagle.Service
+
+object RefcountedServiceSpec extends Specification with Mockito {
+  "PoolServiceWrapper" should {
+    val service = mock[Service[Any, Any]]
+    val promise = new Promise[Any]
+    service(Matchers.any) returns promise
+    val wrapper = spy(new RefcountedService[Any, Any](service))
+
+    "call release() immediately when no requests have been made" in {
+      there was no(service).release()
+      wrapper.release()
+      there was one(service).release()
+    }
+
+    "call release() after pending request finishes" in {
+      val f = wrapper(123)
+      f.isDefined must beFalse
+      there was one(service)(123)
+
+      wrapper.release()
+      there was no(service).release()
+
+      promise() = Return(123)
+      there was one(service).release()
+      f.isDefined must beTrue
+      f() must be_==(123)
+    }
+  }
+}
+

From b9093772ea3acf8e7b6cafaccb7dac15e7a4551a Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Wed, 2 Feb 2011 18:19:38 -0800
Subject: [PATCH 14/43] add builder option: hostConnectionMaxIdleTime(timeout:
 Duration)

---
 .../finagle/builder/ClientBuilder.scala       | 54 +++++++++++--------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index f4978a5160..ba27b10b3a 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -10,7 +10,7 @@ import org.jboss.netty.channel.socket.nio._
 import org.jboss.netty.handler.ssl._
 import org.jboss.netty.bootstrap.ClientBootstrap
 
-import com.twitter.util.Duration
+import com.twitter.util.{Future, Duration}
 import com.twitter.util.TimeConversions._
 
 import com.twitter.finagle.channel._
@@ -69,6 +69,7 @@ case class ClientBuilder[Req, Rep](
   _hostConnectionCoresize: Option[Int],
   _hostConnectionLimit: Option[Int],
   _hostConnectionIdleTime: Option[Duration],
+  _hostConnectionMaxIdleTime: Option[Duration],
   _sendBufferSize: Option[Int],
   _recvBufferSize: Option[Int],
   _retries: Option[Int],
@@ -88,6 +89,7 @@ case class ClientBuilder[Req, Rep](
     None,               // hostConnectionCoresize
     None,               // hostConnectionLimit
     None,               // hostConnectionIdleTime
+    None,               // hostConnectionMaxIdleTime
     None,               // sendBufferSize
     None,               // recvBufferSize
     None,               // retries
@@ -99,23 +101,24 @@ case class ClientBuilder[Req, Rep](
 
   override def toString() = {
     val options = Seq(
-      "name"                   -> _name,
-      "cluster"                -> _cluster,
-      "protocol"               -> _protocol,
-      "connectionTimeout"      -> Some(_connectionTimeout),
-      "requestTimeout"         -> Some(_requestTimeout),
-      "statsReceiver"          -> _statsReceiver,
-      "loadStatistics"         -> _loadStatistics,
-      "hostConnectionLimit"    -> Some(_hostConnectionLimit),
-      "hostConnectionCoresize" -> Some(_hostConnectionCoresize),
-      "hostConnectionIdleTime" -> Some(_hostConnectionIdleTime),
-      "sendBufferSize"         -> _sendBufferSize,
-      "recvBufferSize"         -> _recvBufferSize,
-      "retries"                -> _retries,
-      "logger"                 -> _logger,
-      "channelFactory"         -> _channelFactory,
-      "tls"                    -> _tls,
-      "startTls"               -> _startTls
+      "name"                      -> _name,
+      "cluster"                   -> _cluster,
+      "protocol"                  -> _protocol,
+      "connectionTimeout"         -> Some(_connectionTimeout),
+      "requestTimeout"            -> Some(_requestTimeout),
+      "statsReceiver"             -> _statsReceiver,
+      "loadStatistics"            -> _loadStatistics,
+      "hostConnectionLimit"       -> Some(_hostConnectionLimit),
+      "hostConnectionCoresize"    -> Some(_hostConnectionCoresize),
+      "hostConnectionIdleTime"    -> Some(_hostConnectionIdleTime),
+      "hostConnectionMaxIdleTime" -> Some(_hostConnectionMaxIdleTime),
+      "sendBufferSize"            -> _sendBufferSize,
+      "recvBufferSize"            -> _recvBufferSize,
+      "retries"                   -> _retries,
+      "logger"                    -> _logger,
+      "channelFactory"            -> _channelFactory,
+      "tls"                       -> _tls,
+      "startTls"                  -> _startTls
     )
 
     "ClientBuilder(%s)".format(
@@ -177,6 +180,9 @@ case class ClientBuilder[Req, Rep](
   def hostConnectionIdleTime(timeout: Duration) =
     copy(_hostConnectionIdleTime = Some(timeout))
 
+  def hostConnectionMaxIdleTime(timeout: Duration) =
+    copy(_hostConnectionMaxIdleTime = Some(timeout))
+
   def retries(value: Int) =
     copy(_retries = Some(value))
 
@@ -251,9 +257,15 @@ case class ClientBuilder[Req, Rep](
 
   private[this] def prepareChannel(service: Service[Req, Rep]) = {
     val protocol = _protocol.get
-    // First the codec, then the protocol.
-    protocol.codec.prepareClientChannel(service) flatMap
-      protocol.prepareChannel _ 
+    var future: Future[Service[Req, Rep]] = null
+
+    future = protocol.codec.prepareClientChannel(service)
+    future = future.flatMap { protocol.prepareChannel(_) }
+    _hostConnectionMaxIdleTime.foreach { idleTime =>
+      future = future.map { new ExpiringService(_, idleTime) }
+    }
+
+    future
   }
 
   def buildFactory(): ServiceFactory[Req, Rep] = {

From 1744724d6a68db50850f27b05815a3f2d22c33e6 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 09:27:11 -0800
Subject: [PATCH 15/43] ChannelService: ignore errors subsequent of a
 connection error.

---
 .../twitter/finagle/channel/ChannelService.scala    |  3 ++-
 .../finagle/channel/ChannelServiceSpec.scala        | 13 +++++++++++++
 .../finagle/thrift/ThriftClientFramedCodec.scala    |  4 +++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
index e09a69057f..0e0775c6f8 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelService.scala
@@ -39,7 +39,7 @@ class ChannelService[Req, Rep](channel: Channel, factory: ChannelServiceFactory[
 
     val replyFuture = currentReplyFuture.getAndSet(null)
     if (replyFuture ne null)
-      replyFuture() = message
+      replyFuture.updateIfEmpty(message)
     else  // spurious reply!
       isHealthy = false
 
@@ -69,6 +69,7 @@ class ChannelService[Req, Rep](channel: Channel, factory: ChannelServiceFactory[
     if (currentReplyFuture.compareAndSet(null, replyFuture)) {
       Channels.write(channel, request) {
         case Error(cause) =>
+          isHealthy = false
           replyFuture.updateIfEmpty(Throw(new WriteException(ChannelException(cause))))
         case _ => ()
       }
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
index 53606af12f..8fdb1e3d63 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelServiceSpec.scala
@@ -47,6 +47,19 @@ object ChannelServiceSpec extends Specification with Mockito {
         messageEvent.getFuture.setFailure(new Exception("doh."))
         future() must throwA(new WriteException(new Exception("doh.")))
       }
+
+      "silently ignore other errors after a downstream write failure" in {
+        messageEvent.getFuture.setFailure(new Exception("doh."))
+        future() must throwA(new WriteException(new Exception("doh.")))        
+
+        val stateEvent = mock[ChannelStateEvent]
+        stateEvent.getState returns ChannelState.OPEN
+        stateEvent.getValue returns java.lang.Boolean.FALSE
+        val context = mock[ChannelHandlerContext]
+        val handler = pipeline.getLast.asInstanceOf[ChannelUpstreamHandler]
+        handler.handleUpstream(context, stateEvent)
+        // (setting the future twice would cause an exception)
+      }
     }
 
     "receive replies" in {
diff --git a/finagle-thrift/src/main/scala/com/twitter/finagle/thrift/ThriftClientFramedCodec.scala b/finagle-thrift/src/main/scala/com/twitter/finagle/thrift/ThriftClientFramedCodec.scala
index 995cabd127..d9bfad83b9 100644
--- a/finagle-thrift/src/main/scala/com/twitter/finagle/thrift/ThriftClientFramedCodec.scala
+++ b/finagle-thrift/src/main/scala/com/twitter/finagle/thrift/ThriftClientFramedCodec.scala
@@ -60,7 +60,9 @@ class ThriftClientFramedCodec extends Codec[ThriftClientRequest, Array[Byte]] {
 
   val serverPipelineFactory = clientPipelineFactory
 
-  override def prepareClientChannel(underlying: Service[ThriftClientRequest, Array[Byte]]) = {
+  override def prepareClientChannel(
+    underlying: Service[ThriftClientRequest, Array[Byte]]) =
+  {
     // Attempt to upgrade the protocol the first time around by
     // sending a magic method invocation.
     val memoryBuffer = new TMemoryBuffer(512)

From 80a0794967441dd2a03b7800fc6971079b3ad850 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 09:49:55 -0800
Subject: [PATCH 16/43] make it so that a the default ChannelFactory can be
 revived after a call to .releaseExternalResources.

this avoids the need for a global finagle shutdown, and allows us to
keep refcounts for graceful shutdown.
---
 .../finagle/builder/ClientBuilder.scala       | 32 ++++++++++++++++---
 .../service/ServiceToChannelHandler.scala     |  3 ++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index ba27b10b3a..66fe738763 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -45,11 +45,33 @@ object ClientBuilder {
   def apply() = new ClientBuilder[Any, Any]
   def get() = apply()
 
-  lazy val defaultChannelFactory =
-    new ReferenceCountedChannelFactory(
-      new NioClientSocketChannelFactory(
-        Executors.newCachedThreadPool(),
-        Executors.newCachedThreadPool()))
+  lazy val defaultChannelFactory = new ReferenceCountedChannelFactory(
+    // A "revivable" and lazy ChannelFactory that allows us to
+    // revive ChannelFactories after they have been released.
+    new ChannelFactory {
+      private[this] def make() =
+        new NioClientSocketChannelFactory(
+          Executors.newCachedThreadPool(),
+          Executors.newCachedThreadPool())
+
+      @volatile private[this] var underlying: ChannelFactory = null
+
+      def newChannel(pipeline: ChannelPipeline) = {
+        if (underlying eq null) synchronized {
+          if (underlying eq null)
+            underlying = make()
+        }
+
+        underlying.newChannel(pipeline)
+      }
+
+      def releaseExternalResources() = synchronized {
+        if (underlying ne null) {
+          underlying.releaseExternalResources()
+          underlying = null
+        }
+      }
+    })
 }
 
 /**
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
index 843cdde56b..745d05d55d 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
@@ -19,6 +19,9 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep])
     val channel = ctx.getChannel
     val message = e.getMessage
 
+
+    // TODO: Release, etc.
+
     try {
       // for an invalid type, the exception would be caught by the
       // SimpleChannelUpstreamHandler.

From 7933e18b638d6daaae00f0406c5d63e997d32c24 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 09:56:30 -0800
Subject: [PATCH 17/43] 1.1.11

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index 71ebf167e9..3d878a73c6 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Tue Feb 01 17:23:18 PST 2011
+#Thu Feb 03 09:54:27 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.11-SNAPSHOT
+project.version=1.1.11
 build.scala.versions=2.8.1
 project.initialize=false

From 13ffa9a8112ccacbf8eaba63f9521bc725473c73 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 09:56:31 -0800
Subject: [PATCH 18/43] 1.1.12-SNAPSHOT

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index 3d878a73c6..db5b919039 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 09:54:27 PST 2011
+#Thu Feb 03 09:56:31 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.11
+project.version=1.1.12-SNAPSHOT
 build.scala.versions=2.8.1
 project.initialize=false

From a736e24c0f8400803903f2f5410788ae2c421630 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 11:06:52 -0800
Subject: [PATCH 19/43] ServiceToChannelHandler:   * propate close events (they
 are releases)   * DI the logger   * specs

---
 .../service/ServiceToChannelHandler.scala     | 36 ++++-----
 .../service/ServiceToChannelHandlerSpec.scala | 74 +++++++++++++++++++
 2 files changed, 93 insertions(+), 17 deletions(-)
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
index 745d05d55d..84a95c71c3 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
@@ -1,5 +1,6 @@
 package com.twitter.finagle.service
 
+import java.util.concurrent.atomic.AtomicBoolean
 import java.util.logging.Logger
 import java.util.logging.Level
 
@@ -10,18 +11,22 @@ import com.twitter.util.{Return, Throw}
 import com.twitter.finagle.util.Conversions._
 import com.twitter.finagle.Service
 
-class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep])
+class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep], log: Logger)
   extends SimpleChannelUpstreamHandler
 {
-  private[this] val log = Logger.getLogger(getClass.getName)
+  def this(service: Service[Req, Rep]) = this(service, Logger.getLogger(getClass.getName))
+  private[this] val isShutdown = new AtomicBoolean(false)
+
+  private[this] def shutdown(ch: Channel) =
+    if (isShutdown.compareAndSet(false, true)) {
+      if (ch.isOpen) ch.close()
+      service.release()
+    }
 
   override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
     val channel = ctx.getChannel
     val message = e.getMessage
 
-
-    // TODO: Release, etc.
-
     try {
       // for an invalid type, the exception would be caught by the
       // SimpleChannelUpstreamHandler.
@@ -32,14 +37,18 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep])
 
          case Throw(e: Throwable) =>
            log.log(Level.WARNING, e.getMessage, e)
-           Channels.close(channel)
+           shutdown(channel)
        }
     } catch {
       case e: ClassCastException =>
-        Channels.close(channel)
+        shutdown(channel)
     }
   }
 
+  override def channelClosed(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    shutdown(ctx.getChannel)
+  }
+
   /**
    * Catch and silence certain closed channel exceptions to avoid spamming
    * the logger.
@@ -58,16 +67,9 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep])
         Level.WARNING
     }
 
-    log.log(level,
-            Option(cause.getMessage).getOrElse("Exception caught"),
-            cause)
+    log.log(
+      level, Option(cause.getMessage).getOrElse("Exception caught"), cause)
 
-    ctx.getChannel match {
-      case c: Channel
-      if c.isOpen =>
-        Channels.close(c)
-      case _ =>
-        ()
-    }
+    shutdown(ctx.getChannel)
   }
 }
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala
new file mode 100644
index 0000000000..b02831c631
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala
@@ -0,0 +1,74 @@
+package com.twitter.finagle.service
+
+import java.util.logging.Logger
+
+import org.specs.Specification
+import org.specs.mock.Mockito
+import org.mockito.{Matchers, ArgumentCaptor}
+
+import org.jboss.netty.channel.{
+  ChannelHandlerContext, MessageEvent, Channel,
+  ChannelPipeline, DownstreamMessageEvent}
+
+import com.twitter.util.Future
+import com.twitter.finagle.Service
+
+object ServiceToChannelHandlerSpec extends Specification with Mockito {
+  "ServiceToChannelHandler" should {
+    class Foo { def fooMethod() = "hey there" }
+
+    val log = mock[Logger]
+    val request = new Foo
+    val service = mock[Service[Foo, String]]
+    val handler = new ServiceToChannelHandler(service, log)
+    val pipeline = mock[ChannelPipeline]
+    val channel = mock[Channel]
+    channel.isOpen returns true
+    val ctx = mock[ChannelHandlerContext]
+
+    channel.getPipeline returns pipeline
+    ctx.getChannel returns channel
+    val e = mock[MessageEvent]
+    e.getMessage returns request
+
+    service(Matchers.any[Foo]) answers { f => Future.value(f.asInstanceOf[Foo].fooMethod) }
+
+    "when sending a valid message" in {
+      handler.messageReceived(ctx, e)
+
+      "propagate received messages to the service" in {
+        there was one(service)(request)
+      }
+
+      "write the reply to the channel" in {
+        val captor = ArgumentCaptor.forClass(classOf[DownstreamMessageEvent])
+        there was one(pipeline).sendDownstream(captor.capture)        
+        
+        val dsme = captor.getValue
+        dsme.getMessage must haveClass[String]
+        dsme.getMessage must be_==("hey there")
+      }
+    }
+
+    "service shutdown" in {
+      "when sending an invalid message" in {
+        e.getMessage returns mock[Object]   // wrong type
+        handler.messageReceived(ctx, e)
+
+        // Unfortunately, we rely on catching the ClassCastError from
+        // the invocation of the service itself :-/
+        //   there was no(service)(Matchers.any[Foo])
+        there was one(service).release()
+        there was one(channel).close()
+      }
+
+      "when the service handler throws" in {
+        service(request) returns Future.exception(new Exception("WTF"))
+        handler.messageReceived(ctx, e)
+
+        there was one(service).release()
+        there was one(channel).close()
+      }
+    }
+  }
+}

From 2bb349ba3b5284be4c108f7de44523289ce56dc7 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 15:28:01 -0800
Subject: [PATCH 20/43] generalize the expiry service & use it in the server
 side, too, with a "ChannelClosingHandler" that reliably closes whatever
 channel it is (or will) attached to

---
 .../finagle/builder/ServerBuilder.scala       | 29 ++++++++--
 .../channel/ChannelClosingHandler.scala       | 46 ++++++++++++++++
 .../finagle/service/ExpiringService.scala     | 29 ++++++----
 .../service/ServiceToChannelHandler.scala     |  5 ++
 .../com/twitter/finagle/util/Timer.scala      |  6 +--
 .../channel/ChannelClosingHandlerSpec.scala   | 54 +++++++++++++++++++
 6 files changed, 153 insertions(+), 16 deletions(-)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
index 05f9c3ef97..727cfe6b89 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
@@ -9,17 +9,19 @@ import javax.net.ssl.SSLContext
 
 import org.jboss.netty.bootstrap.ServerBootstrap
 import org.jboss.netty.channel._
+import org.jboss.netty.handler.timeout.IdleStateHandler
 import org.jboss.netty.handler.ssl._
 import org.jboss.netty.channel.socket.nio._
 
-import com.twitter.util.TimeConversions._
+import com.twitter.util.Duration
+import com.twitter.conversions.time._
 
 import com.twitter.finagle._
 import com.twitter.finagle.util.Conversions._
-import channel.{Job, QueueingChannelHandler}
+import channel.{Job, QueueingChannelHandler, ChannelClosingHandler}
 import com.twitter.finagle.util._
 import com.twitter.util.{Future, Promise, Return}
-import service.{StatsFilter, ServiceToChannelHandler}
+import service.{StatsFilter, ServiceToChannelHandler, ExpiringService}
 import stats.{StatsReceiver}
 
 trait Server {
@@ -51,6 +53,7 @@ case class ServerBuilder[Req, Rep](
   _startTls: Boolean,
   _channelFactory: Option[ChannelFactory],
   _maxConcurrentRequests: Option[Int],
+  _hostConnectionMaxIdleTime: Option[Duration],
   _maxQueueDepth: Option[Int])
 {
   import ServerBuilder._
@@ -67,6 +70,7 @@ case class ServerBuilder[Req, Rep](
     false,             // startTls
     None,              // channelFactory
     None,              // maxConcurrentRequests
+    None,              // hostConnectionMaxIdleTime
     None               // maxQueueDepth
   )
 
@@ -98,6 +102,9 @@ case class ServerBuilder[Req, Rep](
   def maxConcurrentRequests(max: Int) =
     copy(_maxConcurrentRequests = Some(max))
 
+  def hostConnectionMaxIdleTime(howlong: Duration) =
+    copy(_hostConnectionMaxIdleTime = Some(howlong))
+
   def maxQueueDepth(max: Int) =
     copy(_maxQueueDepth = Some(max))
 
@@ -142,8 +149,22 @@ case class ServerBuilder[Req, Rep](
         var service = codec.wrapServerChannel(serviceFactory())
         if (_statsReceiver.isDefined)
           service = new StatsFilter(_statsReceiver.get).andThen(service)
-        pipeline.addLast("service", new ServiceToChannelHandler(service))
 
+        // We add the idle time after the codec. This ensures that a
+        // client couldn't DoS us by sending lots of little messages
+        // that don't produce a request object for some time. In other
+        // words, the idle time refers to the idle time from the view
+        // of the protocol.
+        _hostConnectionMaxIdleTime foreach { duration =>
+          val closingHandler = new ChannelClosingHandler
+          pipeline.addLast("closingHandler", closingHandler)
+
+          service = new ExpiringService(service, duration) {
+            override def didExpire() { closingHandler.close() }
+          }
+        }
+
+        pipeline.addLast("service", new ServiceToChannelHandler(service))
         pipeline
       }
     })
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
new file mode 100644
index 0000000000..948142cab6
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
@@ -0,0 +1,46 @@
+package com.twitter.finagle.channel
+
+/**
+ * A Netty channel handler that reliably closes its underlying
+ * connection (when it exists).
+ */
+
+import org.jboss.netty.channel.{
+  SimpleChannelUpstreamHandler, LifeCycleAwareChannelHandler,
+  ChannelHandlerContext, ChannelStateEvent, Channel}
+
+class ChannelClosingHandler
+  extends SimpleChannelUpstreamHandler
+  with LifeCycleAwareChannelHandler
+{
+  private[this] var channel: Channel = null
+  private[this] var awaitingClose = false
+
+  private[this] def setChannel(ch: Channel) = synchronized {
+    channel = ch
+    if (awaitingClose)
+      channel.close()
+  }
+
+  def close() = synchronized {
+    if (channel ne null)
+      channel.close()
+    else
+      awaitingClose = true
+  }
+
+  override def beforeAdd(ctx: ChannelHandlerContext) {
+    if (!ctx.getPipeline.isAttached)
+      return
+
+    setChannel(ctx.getChannel)
+  }
+
+  def afterAdd(ctx: ChannelHandlerContext) {/*nop*/}
+  def beforeRemove(ctx: ChannelHandlerContext) {/*nop*/}
+  def afterRemove(ctx: ChannelHandlerContext) {/*nop*/}
+
+  override def channelOpen(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    setChannel(ctx.getChannel)
+  }
+}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
index 5b9868e351..df59dd91d5 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
@@ -15,15 +15,26 @@ class ExpiringService[Req, Rep](
   private[this] var requestCount = 0
   private[this] var expired = false
   private[this] var task: Option[com.twitter.util.TimerTask] =
-    Some(timer.schedule(maxIdleTime.fromNow) { doExpire() })
-
-  private[this] def doExpire() = synchronized {
-    // We check requestCount == 0 here to avoid the race between
-    // cancellation & running of the timer.
-    if (!expired && requestCount == 0) {
-      underlying.release()
-      expired = true
+    Some(timer.schedule(maxIdleTime.fromNow) { maybeExpire() })
+
+  private[this] def maybeExpire() = {
+    val justExpired = synchronized {
+      // We check requestCount == 0 here to avoid the race between
+      // cancellation & running of the timer.
+      if (!expired && requestCount == 0) {
+        expired = true
+        true
+      } else {
+        false
+      }
     }
+
+    if (justExpired) didExpire()
+  }
+
+  // May be overriden to provide your own expiration action.
+  protected def didExpire() {
+    underlying.release()
   }
 
   def apply(request: Req): Future[Rep] = synchronized {
@@ -44,7 +55,7 @@ class ExpiringService[Req, Rep](
         requestCount -= 1
         if (requestCount == 0) {
           require(!task.isDefined)
-          task = Some(timer.schedule(maxIdleTime.fromNow) { doExpire() })
+          task = Some(timer.schedule(maxIdleTime.fromNow) { maybeExpire() })
         }
       }
     }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
index 84a95c71c3..0b5903efcd 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
@@ -41,6 +41,11 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep], log: Logger)
        }
     } catch {
       case e: ClassCastException =>
+        log.log(
+          Level.SEVERE,
+          "Got ClassCastException while processing a " +
+          "message. This is a codec bug. %s".format(e))
+
         shutdown(channel)
     }
   }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
index 664cccd428..cb6472ee03 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
@@ -5,8 +5,8 @@ import com.twitter.util.{Time, Duration, TimerTask}
 import org.jboss.netty.util.{HashedWheelTimer, Timeout}
 
 object Timer {
-  implicit lazy val default =
-    new Timer(new HashedWheelTimer(10, TimeUnit.MILLISECONDS))
+  val defaultNettyTimer = new HashedWheelTimer(10, TimeUnit.MILLISECONDS)
+  implicit lazy val default = new Timer(defaultNettyTimer)
 }
 
 class Timer(underlying: org.jboss.netty.util.Timer) extends com.twitter.util.Timer 
@@ -33,4 +33,4 @@ class Timer(underlying: org.jboss.netty.util.Timer) extends com.twitter.util.Tim
   private[this] def toTimerTask(task: Timeout) = new TimerTask {
     def cancel() { task.cancel() }
   }
-}
\ No newline at end of file
+}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala
new file mode 100644
index 0000000000..3f683bc445
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala
@@ -0,0 +1,54 @@
+package com.twitter.finagle.channel
+
+import org.specs.Specification
+import org.specs.mock.Mockito
+
+import org.jboss.netty.channel.{
+  Channel, ChannelHandlerContext, ChannelStateEvent,
+  ChannelPipeline}
+
+object ChannelClosingHandlerSpec extends Specification with Mockito {
+  "ChannelClosingHandler" should {
+    val channel = mock[Channel]
+    val handler = new ChannelClosingHandler
+    val ctx = mock[ChannelHandlerContext]
+    val e = mock[ChannelStateEvent]
+    val pipeline = mock[ChannelPipeline]
+
+    pipeline.isAttached returns true
+
+    ctx.getPipeline returns pipeline
+    ctx.getChannel returns channel
+
+    "close the channel immediately" in {
+      "channel is already open" in {
+        handler.channelOpen(ctx, e)
+        there was no(channel).close()
+        handler.close()
+        there was one(channel).close()
+      }
+
+      "channel is attached" in {
+        handler.beforeAdd(ctx)
+        there was no(channel).close()
+        handler.close()
+        there was one(channel).close()
+      }
+    }
+
+    "delay closing until it has been opened" in {
+      handler.close()
+      there was no(channel).close()
+
+      "before channel has been opened" in {
+        handler.channelOpen(ctx, e)
+        there was one(channel).close()
+      }
+
+      "before channel has been attached" in {
+        handler.beforeAdd(ctx)
+        there was one(channel).close()        
+      }
+    }
+  }
+}

From 24f1a4b17ddd8c04577f7e3761e1d677ee0e503f Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 15:36:38 -0800
Subject: [PATCH 21/43] add .requestTimeout() in the ServerBuilder -- note that
 the only thing we can do generically is to close the connection.

thus, most servers will actually want to implement timeouts in their
service stack, producing appropriate timeout messages.
---
 .../com/twitter/finagle/builder/ClientBuilder.scala   |  2 +-
 .../com/twitter/finagle/builder/ServerBuilder.scala   | 11 ++++++++++-
 .../finagle/channel/ChannelClosingHandler.scala       |  4 ++--
 .../com/twitter/finagle/service/TimeoutFilter.scala   |  2 +-
 .../twitter/finagle/service/TimeoutFilterSpec.scala   |  2 +-
 5 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index 66fe738763..fb5b9ae658 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -316,7 +316,7 @@ case class ClientBuilder[Req, Rep](
       factory = buildPool(factory)
 
       if (_requestTimeout < Duration.MaxValue) {
-        val filter = new TimeoutFilter[Req, Rep](Timer.default, _requestTimeout)
+        val filter = new TimeoutFilter[Req, Rep](_requestTimeout)
         factory = filter andThen factory
       }
 
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
index 727cfe6b89..e3022b4648 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
@@ -21,7 +21,7 @@ import com.twitter.finagle.util.Conversions._
 import channel.{Job, QueueingChannelHandler, ChannelClosingHandler}
 import com.twitter.finagle.util._
 import com.twitter.util.{Future, Promise, Return}
-import service.{StatsFilter, ServiceToChannelHandler, ExpiringService}
+import service.{StatsFilter, ServiceToChannelHandler, ExpiringService, TimeoutFilter}
 import stats.{StatsReceiver}
 
 trait Server {
@@ -54,6 +54,7 @@ case class ServerBuilder[Req, Rep](
   _channelFactory: Option[ChannelFactory],
   _maxConcurrentRequests: Option[Int],
   _hostConnectionMaxIdleTime: Option[Duration],
+  _requestTimeout: Option[Duration],
   _maxQueueDepth: Option[Int])
 {
   import ServerBuilder._
@@ -71,6 +72,7 @@ case class ServerBuilder[Req, Rep](
     None,              // channelFactory
     None,              // maxConcurrentRequests
     None,              // hostConnectionMaxIdleTime
+    None,              // requestTimeout
     None               // maxQueueDepth
   )
 
@@ -105,6 +107,9 @@ case class ServerBuilder[Req, Rep](
   def hostConnectionMaxIdleTime(howlong: Duration) =
     copy(_hostConnectionMaxIdleTime = Some(howlong))
 
+  def requestTimeout(howlong: Duration) =
+    copy(_requestTimeout = Some(howlong))
+
   def maxQueueDepth(max: Int) =
     copy(_maxQueueDepth = Some(max))
 
@@ -164,6 +169,10 @@ case class ServerBuilder[Req, Rep](
           }
         }
 
+        _requestTimeout foreach { duration =>
+          service = (new TimeoutFilter(duration)) andThen service
+        }
+
         pipeline.addLast("service", new ServiceToChannelHandler(service))
         pipeline
       }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
index 948142cab6..861b85102f 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
@@ -36,9 +36,9 @@ class ChannelClosingHandler
     setChannel(ctx.getChannel)
   }
 
-  def afterAdd(ctx: ChannelHandlerContext) {/*nop*/}
+  def afterAdd(ctx: ChannelHandlerContext)     {/*nop*/}
   def beforeRemove(ctx: ChannelHandlerContext) {/*nop*/}
-  def afterRemove(ctx: ChannelHandlerContext) {/*nop*/}
+  def afterRemove(ctx: ChannelHandlerContext)  {/*nop*/}
 
   override def channelOpen(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
     setChannel(ctx.getChannel)
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
index 445182d4f9..ea0dc03870 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
@@ -11,7 +11,7 @@ import com.twitter.finagle.{Filter, Service}
  * A filter to apply a global timeout to the request. This allows,
  * e.g., for a server to apply a global request timeout.
  */
-class TimeoutFilter[Req, Rep](timer: Timer, timeout: Duration)
+class TimeoutFilter[Req, Rep](timeout: Duration, timer: Timer = Timer.default)
   extends Filter[Req, Rep, Req, Rep]
 {
   def apply(request: Req, service: Service[Req, Rep]): Future[Rep] = {
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
index dce8d75ba1..03fdea3029 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
@@ -15,7 +15,7 @@ object TimeoutFilterSpec extends Specification with Mockito {
     val service = new Service[String, String] {
       def apply(request: String) = promise
     }
-    val timeoutFilter = new TimeoutFilter[String, String](timer, 1.second)
+    val timeoutFilter = new TimeoutFilter[String, String](1.second)
     val timeoutService = timeoutFilter.andThen(service)
 
     "cancels the request when the service succeeds" in {

From ef6007b7576912f4e797a860c45ac168b3c72c51 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 15:40:05 -0800
Subject: [PATCH 22/43] 1.1.12

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index db5b919039..f26ecc974e 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 09:56:31 PST 2011
+#Thu Feb 03 15:38:59 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.12-SNAPSHOT
+project.version=1.1.12
 build.scala.versions=2.8.1
 project.initialize=false

From f08225936748db07ffb43cab5186937b188901fa Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 15:40:05 -0800
Subject: [PATCH 23/43] 1.1.13-SNAPSHOT

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index f26ecc974e..0fe3a85970 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 15:38:59 PST 2011
+#Thu Feb 03 15:40:05 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.12
+project.version=1.1.13-SNAPSHOT
 build.scala.versions=2.8.1
 project.initialize=false

From 8eeb391b4cc66074243c2a353eba2ab9413a8bee Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:02:59 -0800
Subject: [PATCH 24/43] util 1.6.0

---
 project/build/Project.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/project/build/Project.scala b/project/build/Project.scala
index 9a9134ecdf..761a6780ec 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -71,7 +71,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
     val nettyRepo =
       "repository.jboss.org" at "http://repository.jboss.org/nexus/content/groups/public/"
     val netty     = "org.jboss.netty"      %  "netty"     % "3.2.3.Final"
-    val util      = "com.twitter"          %  "util"      % "1.5.3"
+    val util      = "com.twitter"          %  "util"      % "1.6.0"
 
     val mockito   = "org.mockito"             % "mockito-all" % "1.8.5" % "test" withSources()
     val specs     = "org.scala-tools.testing" % "specs_2.8.0" % "1.6.5" % "test" withSources()

From c5a6889a1acccaaaf01b11be2b2bc8c7a4a174bc Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:04:56 -0800
Subject: [PATCH 25/43] 1.1.13

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index 0fe3a85970..a63edb4734 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 15:40:05 PST 2011
+#Thu Feb 03 16:03:36 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.13-SNAPSHOT
+project.version=1.1.13
 build.scala.versions=2.8.1
 project.initialize=false

From ffb196c0943ea88122c1d58fe231196425a8f4f1 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:04:57 -0800
Subject: [PATCH 26/43] 1.1.14-SNAPSHOT

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index a63edb4734..7223d40944 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 16:03:36 PST 2011
+#Thu Feb 03 16:04:57 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.13
+project.version=1.1.14-SNAPSHOT
 build.scala.versions=2.8.1
 project.initialize=false

From 52c300ebf2caecef160d086cf14315f970b88059 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:53:07 -0800
Subject: [PATCH 27/43] util 1.6.1

---
 project/build/Project.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/project/build/Project.scala b/project/build/Project.scala
index 761a6780ec..970d5ff207 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -71,7 +71,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
     val nettyRepo =
       "repository.jboss.org" at "http://repository.jboss.org/nexus/content/groups/public/"
     val netty     = "org.jboss.netty"      %  "netty"     % "3.2.3.Final"
-    val util      = "com.twitter"          %  "util"      % "1.6.0"
+    val util      = "com.twitter"          %  "util"      % "1.6.1"
 
     val mockito   = "org.mockito"             % "mockito-all" % "1.8.5" % "test" withSources()
     val specs     = "org.scala-tools.testing" % "specs_2.8.0" % "1.6.5" % "test" withSources()

From 725bf545a4334cd196922be51f702c48b597881f Mon Sep 17 00:00:00 2001
From: wilhelm bierbaum <wilhelm@twitter.com>
Date: Thu, 3 Feb 2011 16:55:07 -0800
Subject: [PATCH 28/43] Add official finagle logo

---
 doc/finagle.png | Bin 0 -> 2958 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 doc/finagle.png

diff --git a/doc/finagle.png b/doc/finagle.png
new file mode 100644
index 0000000000000000000000000000000000000000..8ee422a31b06780d648f030d6678de567817ebb2
GIT binary patch
literal 2958
zcmV;93vu*`P)<h;3K|Lk000e1NJLTq001xm001xn0{{R3ZOm)L000UvX+uL$Nkc;*
zaB^>EX>4Tx0C?J+mUmQC*A|D*y?1({%`nm#dXp|Nfb=dP9RyJrW(F9_0K*JTY>22p
zL=h1IMUbF?0i&TvtcYSED5zi$NDxqBFp8+CWJcCXe0h2A<>mLsz2Dkr?{oLrd!Mx~
z03=TzE-wX^0w9?u;0Jm*(^rK@(6Rjh26%u0rT{Qm>8ZX!?!iDLFE<x@y2uIqi{1<Y
zNc_HK=;=?Vga1#`tW>@L0LWj&=4?(nOT_siPRbOditRHZrp6?S8AgejFG^6va$=5K
z<fWf|7THnE>|`EW#NwP&*~x4%_lS6VhL9s-#7D#h8C*`Lh;NHnGf9}t74chfY%+(L
z4giWIwhK6{coCb3n8XhbbP@4#0C1$ZFF5847I3lz;zPNlq-OKEaq$AWE=!MYYHiJ+
zdvY?9I0Av8Ka-Wn<g@86Daol!UN!)WXZ|c1ac$|MB3qhTTUr{L8JT`jsQ<e7Hzn@v
zBE1Uu+%t&Q_lNDT{8H)wV9bhYv+ECA%zgkmwgMn`{|}qyApj&reQUq*#d&Drd5ISY
zQf-WlGcz-dxEz*|xS+r5e>(gPeepdb@piwLhwjRWWeSr7baCBSDM=|pK0Q5^$>Pur
z|2)M1IPkCYSQ^NQ<?uN?QADU{%DB8ZQM-9;u7I1uqjP!xsfqtE>`z*pYmq4Rp8z$=
z2uR(a0_5jDfT9oq5_wSE_22vEgAWDbn-``!u{igi1^xT3aEbVl&W-yV=Mor9X9@Wk
zi)-R*3DAH5Bmou30~MeFbb%o-16IHmI084Y0{DSo5DwM?7KjJQfDbZ3F4znTKoQsl
z_JT@K1L{E|XaOfc2RIEbfXm=IxC!on2Vew@gXdrdyaDqN1YsdEM1kZXRY(gmfXpBU
zWDmJPK2RVO4n;$85DyYUxzHA<2r7jtp<1XB`W89`U4X7a1JFHa6q<s5h2FymOoFMf
zGOP_7!wlF7_J)JuHE<l92Is)}@J_e_u7i)k?eGQoI(!EnfuF;(2tbGk4N*f35eDLd
z_#qKUEW$@NAcaUdQirr4T}Ur-3mHMCk#{Hzih`n}3{kcyPgDqsg-SzhKoz4ZQAbhj
zs2<cU)F^5O^$ATzE1?b0HfS&ODs&t=6J3BVM>n9`(3jA6(BtSg7z~Dn(ZN_@JTc*z
z1k5^2G3EfK6>}alfEmNgVzF3xtO3>z>xX4x1=s@Ye(W*qIqV>I9QzhW#Hr%UaPGJW
z91oX=E5|kA&f*4f6S#T26kZE&gZIO;@!9wid_BGke*-^`pC?EYbO?5YU_t_6Gogae
zLbybDNO(mg64i;;!~i0fxQSRnJWjkq93{RZ$&mC(E~H43khGI@gmj*CkMxR6CTo)&
z$q{4$c_+D%e3AT^{8oY@VI<)t!Is!4Q6<qXF(~mu5-+JG=_I*UGDosp@}%Sq$!RIP
zl(v+M6jN%0RF%{zsbQ&EX^OO|w4Zdcbg^`k^i}Ce8LW)9jGGKwCST^T%te_o3PRDK
zxKLP>EtGo7CCWGzL)D>rQ4^>|)NiQ$)EQYB*=4e!vRSfKvS(yRXb4T4=0!`QmC#Pm
zhG_4XC@*nZ!dbFoNz0PKC3A9$a*lEwxk9;CxjS<2<>~Tn@`>`h<vZjbDWDYe6#^78
z6%Hy~QkYhxD%vWt6bltkDBf3smGqSYmDVX8R_arlRaQ~<P)=3euY6H?T7{<KsFI*k
zrgBzgN|mB&ugX;|Q$45pj4n%eq9@TS=solqH6=AqHKAIqTEE)7x{i8?dY*c#`Xdd3
z216rOqfDb)V@6X|(^oTBvsv@L7G8^?6|c2Vt5<7ITSq%gdz*HL_N0!Sj+ai3PP5KK
zU9zr&ZkleL?rlAc9z!ot?||M-eOTW@KVH8||Aql<U}?ZLIAAca6us1XDQ{`r(qTiA
zp_5^TVYA_=5zWZQD9@<F=!LPSafI=1<6h%WCKe`1CiNx{Ol3@0nC6*wnf_{~Z^kmK
zGP`X~Hg`AQXx?f5a+$$0&a#8c?pjbRd@Z(FbX$D1w6f$|wpdPCX<9{FRa*@+s0@Eb
zG2@Cg+S=KAqxEU)cQ%$b0-F;yzt|euCfYXHPA=D3&RJf+e9TVWj%inGH)2n>kG4N#
zKjNU~z;vi{c;cwx$aZXSoN&@}N^m;n^upQ1neW`@Jm+HLvfkyqE8^<mTIkyECgT?3
zR_XTGUEMv-z1e-n!@^^o$9Ye*r?=;B&tWfRFP2xM*USp573){@c$2(?yeqw*_~`ra
zeY$*M-xa=ld>^jVTFG14;RpP@{Py@g^4IZC^Zz~o6W||E74S6BG%z=?H;57x71R{;
zCfGT+B=|vyZiq0XJ5(|>GPE&tF3dHoG;Cy*@v8N!u7@jxbHh6$uo0mV4H2`e-B#~i
zJsxQhSr9q2MrTddnyYIS)+Vhz6D1kNj5-;Ojt+}%ivGa#W7aWeW4vOjV`f+`tbMHK
zY)5t(dx~SnDdkMW+QpW}PR7~A?TMR;cZe^KpXR!7E4eQdJQHdX<`Vr9k0dT6g(bBn
z<C3G3Pw`}UiM*Z^m6WWMfmDOkg4B^To3y=YGkkA;LpqecCcRTY75z;033Y{Ag`*kv
z8C4l?Gea{^W=Uu9vih?1vv*`q<hbX2y$-dGwXQo?Eq8P7=z6F1wHu%fF&nx!YHZBk
zIKIha)6va@&54_T$TP_+&3nBiY)e<Za{i|Lv8^6kn+qfg_yxn;Y`4{HM{VbB@84m*
zWB-m%h3vv>MJ7e%MIVY;#n-+v{i@=tg`KfG`%5fK4(`J2;_VvR?Xdf3sdQ;h>DV6M
zJ?&-mvcj_0d!zPVEnik%vyZS(xNoGwr=oMe=Kfv#KUBt7-l=k~YOPkP-cdbwfPG-_
zpyR=o8s(azn)ipehwj#T)V9}Y*Oec}9L_lWv_7=H_iM)2jSUJ7MGYU1@Q#ce4LsV@
zXw}%*q|{W>3^xm#r;bG)yZMdlH=QkpEw!z*)}rI!xbXP1Z==5*I^lhy`y}IJ%XeDe
zRku;v3frOf?Dm<C_>Pgz@Xmo#D^7KH*><&kZ}k0<(`u)y&d8oAIZHU3e|F(q&bit1
zspqFJ#9bKcj_Q7Jan;4!Jpn!am%J}sx$J)VVy{#0xhr;8PG7aTdg>bETE}(E>+O9O
zeQiHj{Lt2K+24M{>PF{H>ziEz%LmR5It*U8<$CM#ZLizc@2tEtFcdO$cQ|r*<SzT}
z<h`VOFYjmEpMS9FA^KtABdJH_kCh(R{iye2>xkvZnNio#z9&IX9*nWZp8u5o(}(f=
zr{t&Q6RH!9lV+2rr`)G*K3n~4{CVp0`RRh6rGKt|q5I;yUmSnwn^`q8{*wQ4;n(6<
z@~@7(UiP|s)_?Z#o8&k1bA@l^-yVI(c-Q+r?ES=i<_GMDijR69yFPh;dbp6hu<#rA
zg!B97KMrI7000J1OjJex|Nj6009C@%egFUf32;bRa{vGf6951U69E94oEQKA00(qQ
zO+^RW0T2i;HVWayF8}}l;7LS5R2b7^U_gL>Y+#BFLjC^-A{iT)z?A(jFy+7nqL>r7
zAUYt_0Wih*0GS7sV-kR9Fo4T3{|AEK42&S^2LsCkpr||pPYQz|g9HOl{-WJ8ZZI&U
zC<&@c04c4Bnm|Dyr3j>wr}WH~VBkmqY5>Y5J1{6nFt8*sfGD2yxW~(YR8o-0N}ya)
z(3kaKYRU?*;OFHM3^G7{4PfoeAngS}Q!E%5?El&S|G>Z@|G)nK0|ptOzrcnwI{W~8
z$dLt1ITe8^r7i{r4hDuLKuU&zVI~*Y8BP#?*}(k_0E%WhAXL>?Q2+n{07*qoM6N<$
Ef<07_!vFvP

literal 0
HcmV?d00001


From e9db8cabbe24604f65cef578cf67d63b314acfd5 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:55:59 -0800
Subject: [PATCH 29/43] 1.1.14

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index 7223d40944..0d02f8c390 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 16:04:57 PST 2011
+#Thu Feb 03 16:53:32 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.14-SNAPSHOT
+project.version=1.1.14
 build.scala.versions=2.8.1
 project.initialize=false

From 021c9cc79669b3225962657687c672041985fd0f Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 16:55:59 -0800
Subject: [PATCH 30/43] 1.1.15-SNAPSHOT

---
 project/build.properties | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/project/build.properties b/project/build.properties
index 0d02f8c390..694b2cda41 100644
--- a/project/build.properties
+++ b/project/build.properties
@@ -1,8 +1,8 @@
 #Project properties
-#Thu Feb 03 16:53:32 PST 2011
+#Thu Feb 03 16:55:59 PST 2011
 project.organization=com.twitter
 project.name=finagle
 sbt.version=0.7.4
-project.version=1.1.14
+project.version=1.1.15-SNAPSHOT
 build.scala.versions=2.8.1
 project.initialize=false

From 70a819454e8b39767b20980db867f2fb4f678ccf Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 17:30:38 -0800
Subject: [PATCH 31/43] ServiceToChannelHandler => channel

---
 .../finagle/{service => channel}/ServiceToChannelHandler.scala  | 2 +-
 .../{service => channel}/ServiceToChannelHandlerSpec.scala      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename finagle-core/src/main/scala/com/twitter/finagle/{service => channel}/ServiceToChannelHandler.scala (98%)
 rename finagle-core/src/test/scala/com/twitter/finagle/{service => channel}/ServiceToChannelHandlerSpec.scala (98%)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
similarity index 98%
rename from finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
rename to finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
index 0b5903efcd..df14d0b935 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ServiceToChannelHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
@@ -1,4 +1,4 @@
-package com.twitter.finagle.service
+package com.twitter.finagle.channel
 
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.logging.Logger
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
similarity index 98%
rename from finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala
rename to finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
index b02831c631..dc873d756d 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/service/ServiceToChannelHandlerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
@@ -1,4 +1,4 @@
-package com.twitter.finagle.service
+package com.twitter.finagle.channel
 
 import java.util.logging.Logger
 

From da751caee3dd6458d755017d2b66f7fc9c134934 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Thu, 3 Feb 2011 17:31:23 -0800
Subject: [PATCH 32/43] bring reference counted factories to the ServerBuilder.

---
 .../finagle/builder/ChannelFactory.scala      | 70 +++++++++++++++++++
 .../finagle/builder/ClientBuilder.scala       | 49 ++-----------
 .../finagle/builder/ServerBuilder.scala       | 34 ++++++---
 3 files changed, 99 insertions(+), 54 deletions(-)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala
new file mode 100644
index 0000000000..f4cb89cc6e
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala
@@ -0,0 +1,70 @@
+package com.twitter.finagle.builder
+
+import org.jboss.netty.channel.{
+  ChannelPipeline, ChannelFactory, ServerChannelFactory,
+  ServerChannel}
+
+class ReferenceCountedChannelFactory(underlying: ChannelFactory)
+  extends ChannelFactory
+{
+  private[this] var refcount = 0
+
+  def acquire() = synchronized {
+    refcount += 1
+  }
+
+  def newChannel(pipeline: ChannelPipeline) = underlying.newChannel(pipeline)
+
+  // TODO: after releasing external resources, we can still use the
+  // underlying factory?  (ie. it'll create new threads, etc.?)
+  def releaseExternalResources() = synchronized {
+    refcount -= 1
+    if (refcount == 0)
+      underlying.releaseExternalResources()
+  }
+}
+
+/**
+ * A "revivable" and lazy ChannelFactory that allows us to revive
+ * ChannelFactories after they have been released.
+ */
+class LazyRevivableChannelFactory(make: () => ChannelFactory)
+  extends ChannelFactory
+{
+  @volatile private[this] var underlying: ChannelFactory = null
+
+  def newChannel(pipeline: ChannelPipeline) = {
+    if (underlying eq null) synchronized {
+      if (underlying eq null)
+        underlying = make()
+    }
+
+    underlying.newChannel(pipeline)
+  }
+
+  def releaseExternalResources() = synchronized {
+    if (underlying ne null) {
+      underlying.releaseExternalResources()
+      underlying = null
+    }
+  }
+}
+
+/**
+ * Yikes. This monstrosity is needed to wrap server channel factories
+ * generically. Netty only checks the actual channel factory types for
+ * servers dynamically, so using a ChannelFactory in the server
+ * results in a runtime error. However, the above wrappers are generic
+ * and should be shared among the different types of factories. We are
+ * in effect exchanging the possibility of one runtime error for
+ * another. Another approach would be to use dynamic proxies, but this
+ * seems a little simpler for such a simple interface.
+ */
+class ChannelFactoryToServerChannelFactory(underlying: ChannelFactory)
+  extends ServerChannelFactory
+{
+  def newChannel(pipeline: ChannelPipeline) =
+    underlying.newChannel(pipeline).asInstanceOf[ServerChannel]
+  def releaseExternalResources() =
+    underlying.releaseExternalResources()
+}
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index fb5b9ae658..6a529ff9e0 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -21,57 +21,16 @@ import com.twitter.finagle.service._
 import com.twitter.finagle.stats.StatsReceiver
 import com.twitter.finagle.loadbalancer.{LoadBalancedFactory, LeastQueuedStrategy}
 
-class ReferenceCountedChannelFactory(underlying: ChannelFactory)
-  extends ChannelFactory
-{
-  private[this] var refcount = 0
-
-  def acquire() = synchronized {
-    refcount += 1
-  }
-
-  def newChannel(pipeline: ChannelPipeline) = underlying.newChannel(pipeline)
-
-  // TODO: after releasing external resources, we can still use the
-  // underlying factory?  (ie. it'll create new threads, etc.?)
-  def releaseExternalResources() = synchronized {
-    refcount -= 1
-    if (refcount == 0)
-      underlying.releaseExternalResources()
-  }
-}
-
 object ClientBuilder {
   def apply() = new ClientBuilder[Any, Any]
   def get() = apply()
 
-  lazy val defaultChannelFactory = new ReferenceCountedChannelFactory(
-    // A "revivable" and lazy ChannelFactory that allows us to
-    // revive ChannelFactories after they have been released.
-    new ChannelFactory {
-      private[this] def make() =
+  lazy val defaultChannelFactory =
+    new ReferenceCountedChannelFactory(
+      new LazyRevivableChannelFactory(() =>
         new NioClientSocketChannelFactory(
           Executors.newCachedThreadPool(),
-          Executors.newCachedThreadPool())
-
-      @volatile private[this] var underlying: ChannelFactory = null
-
-      def newChannel(pipeline: ChannelPipeline) = {
-        if (underlying eq null) synchronized {
-          if (underlying eq null)
-            underlying = make()
-        }
-
-        underlying.newChannel(pipeline)
-      }
-
-      def releaseExternalResources() = synchronized {
-        if (underlying ne null) {
-          underlying.releaseExternalResources()
-          underlying = null
-        }
-      }
-    })
+          Executors.newCachedThreadPool())))
 }
 
 /**
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
index e3022b4648..60681dbdb6 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
@@ -18,10 +18,10 @@ import com.twitter.conversions.time._
 
 import com.twitter.finagle._
 import com.twitter.finagle.util.Conversions._
-import channel.{Job, QueueingChannelHandler, ChannelClosingHandler}
+import channel.{Job, QueueingChannelHandler, ChannelClosingHandler, ServiceToChannelHandler}
 import com.twitter.finagle.util._
 import com.twitter.util.{Future, Promise, Return}
-import service.{StatsFilter, ServiceToChannelHandler, ExpiringService, TimeoutFilter}
+import service.{StatsFilter, ExpiringService, TimeoutFilter}
 import stats.{StatsReceiver}
 
 trait Server {
@@ -32,10 +32,12 @@ object ServerBuilder {
   def apply() = new ServerBuilder[Any, Any]()
   def get() = apply()
 
-  val defaultChannelFactory =
-    new NioServerSocketChannelFactory(
-      Executors.newCachedThreadPool(),
-      Executors.newCachedThreadPool())
+  lazy val defaultChannelFactory =
+    new ReferenceCountedChannelFactory(
+      new LazyRevivableChannelFactory(() =>
+        new NioServerSocketChannelFactory(
+          Executors.newCachedThreadPool(),
+          Executors.newCachedThreadPool())))
 }
 
 // TODO: common superclass between client & server builders for common
@@ -51,7 +53,7 @@ case class ServerBuilder[Req, Rep](
   _logger: Option[Logger],
   _tls: Option[SSLContext],
   _startTls: Boolean,
-  _channelFactory: Option[ChannelFactory],
+  _channelFactory: Option[ReferenceCountedChannelFactory],
   _maxConcurrentRequests: Option[Int],
   _hostConnectionMaxIdleTime: Option[Duration],
   _requestTimeout: Option[Duration],
@@ -90,7 +92,7 @@ case class ServerBuilder[Req, Rep](
   def bindTo(address: SocketAddress) =
     copy(_bindTo = Some(address))
 
-  def channelFactory(cf: ChannelFactory) =
+  def channelFactory(cf: ReferenceCountedChannelFactory) =
     copy(_channelFactory = Some(cf))
 
   def logger(logger: Logger) = copy(_logger = Some(logger))
@@ -120,7 +122,9 @@ case class ServerBuilder[Req, Rep](
       throw new IncompleteSpecification("No codec was specified")
     }
 
-   val bs = new ServerBootstrap(_channelFactory getOrElse defaultChannelFactory)
+   val cf = _channelFactory getOrElse defaultChannelFactory
+   cf.acquire()
+   val bs = new ServerBootstrap(new ChannelFactoryToServerChannelFactory(cf))
 
     bs.setOption("tcpNoDelay", true)
     // bs.setOption("soLinger", 0) // XXX: (TODO)
@@ -173,6 +177,14 @@ case class ServerBuilder[Req, Rep](
           service = (new TimeoutFilter(duration)) andThen service
         }
 
+        // Register the channel so we can wait for them for a
+        // drain. We close the socket but wait for all handlers to
+        // complete (to drain them individually.)  Note: this would be
+        // complicated by the presence of pipelining.
+        
+        // Active request set? 
+
+
         pipeline.addLast("service", new ServiceToChannelHandler(service))
         pipeline
       }
@@ -183,6 +195,10 @@ case class ServerBuilder[Req, Rep](
       def close() = {
         val done = new Promise[Void]
         channel.close() { case _ => done() = Return(null) }
+
+        // Wait for outstanding requests.
+
+        // XXX cf.releaseExternalResources()
         done
       }
     }

From 38b75c66bfec55e33cfd8efb2d80f52c7d58f287 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Fri, 4 Feb 2011 17:07:54 -0800
Subject: [PATCH 33/43] proper draining of server channels.

---
 .../finagle/builder/ChannelFactory.scala      |   2 +-
 .../finagle/builder/ClientBuilder.scala       |   2 +-
 .../finagle/builder/ServerBuilder.scala       | 123 +++++++++++++-----
 .../channel/ChannelClosingHandler.scala       |  20 ++-
 .../channel/ServiceToChannelHandler.scala     |  66 ++++++++--
 .../finagle/service/ExpiringService.scala     |   7 +
 .../channel/ChannelClosingHandlerSpec.scala   |   6 +-
 .../channel/ServiceToChannelHandlerSpec.scala |  10 +-
 .../twitter/finagle/thrift/EndToEndSpec.scala |   6 +-
 .../finagle/thrift/ServiceEndToEndSpec.scala  |   4 +-
 .../ThriftClientFinagleServerSpec.scala       |   2 +-
 project/build/Project.scala                   |   2 +-
 12 files changed, 186 insertions(+), 64 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala
index f4cb89cc6e..3d8b79a323 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ChannelFactory.scala
@@ -19,7 +19,7 @@ class ReferenceCountedChannelFactory(underlying: ChannelFactory)
   // underlying factory?  (ie. it'll create new threads, etc.?)
   def releaseExternalResources() = synchronized {
     refcount -= 1
-    if (refcount == 0)
+    if (refcount <= 0)
       underlying.releaseExternalResources()
   }
 }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index 6a529ff9e0..d496e96e26 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -25,7 +25,7 @@ object ClientBuilder {
   def apply() = new ClientBuilder[Any, Any]
   def get() = apply()
 
-  lazy val defaultChannelFactory =
+  val defaultChannelFactory =
     new ReferenceCountedChannelFactory(
       new LazyRevivableChannelFactory(() =>
         new NioClientSocketChannelFactory(
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
index 60681dbdb6..abec7a36c8 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
@@ -1,6 +1,7 @@
 package com.twitter.finagle.builder
 
 import scala.collection.JavaConversions._
+import scala.collection.mutable.HashSet
 
 import java.net.SocketAddress
 import java.util.concurrent.{Executors, LinkedBlockingQueue}
@@ -20,19 +21,25 @@ import com.twitter.finagle._
 import com.twitter.finagle.util.Conversions._
 import channel.{Job, QueueingChannelHandler, ChannelClosingHandler, ServiceToChannelHandler}
 import com.twitter.finagle.util._
-import com.twitter.util.{Future, Promise, Return}
+import com.twitter.finagle.util.Timer._
+import com.twitter.util.{Future, Promise, Return, Throw}
 import service.{StatsFilter, ExpiringService, TimeoutFilter}
 import stats.{StatsReceiver}
 
 trait Server {
-  def close(): Future[Void]
+  /** 
+   * Close the underlying server gracefully with the given grace
+   * period. close() will drain the current channels, waiting up to
+   * ``timeout'', after which channels are forcibly closed.
+   */ 
+  def close(timeout: Duration = Duration.MaxValue)
 }
 
 object ServerBuilder {
   def apply() = new ServerBuilder[Any, Any]()
   def get() = apply()
 
-  lazy val defaultChannelFactory =
+  val defaultChannelFactory =
     new ReferenceCountedChannelFactory(
       new LazyRevivableChannelFactory(() =>
         new NioServerSocketChannelFactory(
@@ -122,52 +129,67 @@ case class ServerBuilder[Req, Rep](
       throw new IncompleteSpecification("No codec was specified")
     }
 
-   val cf = _channelFactory getOrElse defaultChannelFactory
-   cf.acquire()
-   val bs = new ServerBootstrap(new ChannelFactoryToServerChannelFactory(cf))
-
+    val cf = _channelFactory getOrElse defaultChannelFactory
+    cf.acquire()
+    val bs = new ServerBootstrap(new ChannelFactoryToServerChannelFactory(cf))
+     
     bs.setOption("tcpNoDelay", true)
     // bs.setOption("soLinger", 0) // XXX: (TODO)
     bs.setOption("reuseAddress", true)
     _sendBufferSize foreach { s => bs.setOption("sendBufferSize", s) }
     _recvBufferSize foreach { s => bs.setOption("receiveBufferSize", s) }
+     
+    val queueingChannelHandler = _maxConcurrentRequests map { maxConcurrentRequests =>
+      val maxQueueDepth = _maxQueueDepth.getOrElse(Int.MaxValue)
+      val queue = new LinkedBlockingQueue[Job](maxQueueDepth)
+      new QueueingChannelHandler(maxConcurrentRequests, queue)
+    }
+
+    trait ChannelHandle {
+      def drain(): Future[Unit]
+      def close()
+    }
+
+    val channels = new HashSet[ChannelHandle]
 
     bs.setPipelineFactory(new ChannelPipelineFactory {
       def getPipeline = {
         val pipeline = codec.serverPipelineFactory.getPipeline
 
-        for (maxConcurrentRequests <- _maxConcurrentRequests) {
-          val maxQueueDepth = _maxQueueDepth.getOrElse(Int.MaxValue)
-          val queue = new LinkedBlockingQueue[Job](maxQueueDepth)
-          pipeline.addFirst("queue", new QueueingChannelHandler(maxConcurrentRequests, queue))
-        }
+        queueingChannelHandler foreach { pipeline.addFirst("queue", _) }
 
-        for (logger <- _logger) {
+        _logger foreach { logger =>
           pipeline.addFirst(
             "channelLogger", ChannelSnooper(_name getOrElse "server")(logger.info))
         }
 
         // SSL comes first so that ChannelSnooper gets plaintext
-        for (ctx <- _tls) {
+        _tls foreach { ctx =>
           val sslEngine = ctx.createSSLEngine()
           sslEngine.setUseClientMode(false)
           sslEngine.setEnableSessionCreation(true)
           pipeline.addFirst("ssl", new SslHandler(sslEngine, _startTls))
         }
 
+        // Compose the service stack.
         var service = codec.wrapServerChannel(serviceFactory())
-        if (_statsReceiver.isDefined)
-          service = new StatsFilter(_statsReceiver.get).andThen(service)
+
+        _statsReceiver foreach { statsReceiver =>
+          service = (new StatsFilter(statsReceiver)) andThen service
+        }
 
         // We add the idle time after the codec. This ensures that a
         // client couldn't DoS us by sending lots of little messages
         // that don't produce a request object for some time. In other
         // words, the idle time refers to the idle time from the view
         // of the protocol.
-        _hostConnectionMaxIdleTime foreach { duration =>
-          val closingHandler = new ChannelClosingHandler
-          pipeline.addLast("closingHandler", closingHandler)
 
+        // TODO: can we share closing handler instances with the
+        // channelHandler?
+
+        val closingHandler = new ChannelClosingHandler
+        pipeline.addLast("closingHandler", closingHandler)
+        _hostConnectionMaxIdleTime foreach { duration =>
           service = new ExpiringService(service, duration) {
             override def didExpire() { closingHandler.close() }
           }
@@ -181,25 +203,68 @@ case class ServerBuilder[Req, Rep](
         // drain. We close the socket but wait for all handlers to
         // complete (to drain them individually.)  Note: this would be
         // complicated by the presence of pipelining.
-        
-        // Active request set? 
 
+        // serialize requests?
+
+        val channelHandler = new ServiceToChannelHandler(service)
+
+        val handle = new ChannelHandle {
+          def close() =
+            channelHandler.close()
+          def drain() = {
+            channelHandler.drain()
+            channelHandler.onShutdown
+          }
+        }
+
+        channels.synchronized { channels += handle }
+        channelHandler.onShutdown ensure {
+          channels.synchronized {
+            channels.remove(handle)
+          }
+        }
 
-        pipeline.addLast("service", new ServiceToChannelHandler(service))
+        pipeline.addLast("channelHandler", channelHandler)
         pipeline
       }
     })
 
-    val channel = bs.bind(_bindTo.get)
+    val serverChannel = bs.bind(_bindTo.get)
     new Server {
-      def close() = {
-        val done = new Promise[Void]
-        channel.close() { case _ => done() = Return(null) }
+      def close(timeout: Duration = Duration.MaxValue) = {
+        // According to NETTY-256, the following sequence of operations
+        // has no race conditions.
+        //
+        //   - close the server socket  (awaitUninterruptibly)
+        //   - close all open channels  (awaitUninterruptibly)
+        //   - releaseExternalResources 
+        //
+        // We modify this a little bit, to allow for graceful draining,
+        // closing open channels only after the grace period.
+        //
+        // The next step here is to do a half-closed socket: we want to
+        // suspend reading, but not writing to a socket.  This may be
+        // important for protocols that do any pipelining, and may
+        // queue in their codecs.
+     
+        // On cursory inspection of the relevant Netty code, this
+        // should never block (it is little more than a close() syscall
+        // on the FD).
+        serverChannel.close().awaitUninterruptibly()
+
+        // At this point, no new channels may be created.
+        val joined = channels.synchronized {
+          Future.join(channels.toSeq map { _.drain() })
+        }
+
+        // Wait for all channels to shut down.
+        joined.get(timeout)
 
-        // Wait for outstanding requests.
+        // Force close any remaining connections. Don't wait for
+        // success.
+        channels.synchronized { channels foreach { _.close() } }
 
-        // XXX cf.releaseExternalResources()
-        done
+        bs.releaseExternalResources()
       }
     }
   }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
index 861b85102f..8113073094 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ChannelClosingHandler.scala
@@ -9,31 +9,36 @@ import org.jboss.netty.channel.{
   SimpleChannelUpstreamHandler, LifeCycleAwareChannelHandler,
   ChannelHandlerContext, ChannelStateEvent, Channel}
 
+import com.twitter.finagle.util.Conversions._
+import com.twitter.finagle.util.LatentChannelFuture
+
 class ChannelClosingHandler
   extends SimpleChannelUpstreamHandler
   with LifeCycleAwareChannelHandler
 {
+  private[this] val channelCloseFuture = new LatentChannelFuture
   private[this] var channel: Channel = null
   private[this] var awaitingClose = false
 
   private[this] def setChannel(ch: Channel) = synchronized {
     channel = ch
+    channelCloseFuture.setChannel(ch)
     if (awaitingClose)
-      channel.close()
+      channel.close() proxyTo channelCloseFuture
   }
 
   def close() = synchronized {
-    if (channel ne null)
+    if (channel ne null) {
       channel.close()
-    else
+    } else {
       awaitingClose = true
+      channelCloseFuture
+    }
   }
 
   override def beforeAdd(ctx: ChannelHandlerContext) {
-    if (!ctx.getPipeline.isAttached)
-      return
-
-    setChannel(ctx.getChannel)
+    if (ctx.getPipeline.isAttached)
+      setChannel(ctx.getChannel)
   }
 
   def afterAdd(ctx: ChannelHandlerContext)     {/*nop*/}
@@ -42,5 +47,6 @@ class ChannelClosingHandler
 
   override def channelOpen(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
     setChannel(ctx.getChannel)
+    super.channelOpen(ctx, e)
   }
 }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
index df14d0b935..1678cd4fb9 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/ServiceToChannelHandler.scala
@@ -6,38 +6,76 @@ import java.util.logging.Level
 
 import org.jboss.netty.channel._
 
-import com.twitter.util.{Return, Throw}
+import com.twitter.util.{Future, Promise, Return, Throw}
 
 import com.twitter.finagle.util.Conversions._
 import com.twitter.finagle.Service
 
 class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep], log: Logger)
-  extends SimpleChannelUpstreamHandler
+  extends ChannelClosingHandler
 {
   def this(service: Service[Req, Rep]) = this(service, Logger.getLogger(getClass.getName))
-  private[this] val isShutdown = new AtomicBoolean(false)
 
-  private[this] def shutdown(ch: Channel) =
-    if (isShutdown.compareAndSet(false, true)) {
-      if (ch.isOpen) ch.close()
+  private[this] val onShutdownPromise = new Promise[Unit]
+  @volatile private[this] var isShutdown = false
+  @volatile private[this] var isDraining = false
+  @volatile private[this] var isIdle = true
+
+  private[this] def shutdown() = synchronized {
+    if (!isShutdown) {
+      isShutdown = true
+      close() onSuccess {
+        onShutdownPromise() = Return(())
+      }
       service.release()
     }
+  }
+
+  val onShutdown: Future[Unit] = onShutdownPromise
+
+  // Admit no new requests.
+  def drain() = synchronized {
+    isDraining = true
+
+    if (isIdle)
+      shutdown()
+  }
 
-  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
+  // TODO: keep busy state?  today, this is the responsibility of the
+  // codec, but this seems icky. as a go-between, we may create a
+  // "serialization" handler in the server pipeline.
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent): Unit = synchronized {
     val channel = ctx.getChannel
     val message = e.getMessage
 
+    if (isDraining) return
+
+    // This is possible if the codec queued messages for us while
+    // draining.
+
     try {
       // for an invalid type, the exception would be caught by the
       // SimpleChannelUpstreamHandler.
-      val req = message.asInstanceOf[Req]
-      service(req) respond {
+      val replyFuture = service(message.asInstanceOf[Req])
+
+      isIdle = false
+
+      // We really want to know when the *write* is done.  That's when
+      // we can drain.
+      replyFuture respond {
          case Return(value) =>
-           Channels.write(ctx.getChannel, value)
+           // Not really idle actually -- we shouldn't *actually* shut
+           // down here..
+           
+           synchronized { isIdle = true }
+
+           Channels.write(channel, value) onSuccessOrFailure {
+             synchronized { if (isDraining) shutdown() }
+           }
 
          case Throw(e: Throwable) =>
            log.log(Level.WARNING, e.getMessage, e)
-           shutdown(channel)
+           shutdown()
        }
     } catch {
       case e: ClassCastException =>
@@ -46,12 +84,12 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep], log: Logger)
           "Got ClassCastException while processing a " +
           "message. This is a codec bug. %s".format(e))
 
-        shutdown(channel)
+        shutdown()
     }
   }
 
   override def channelClosed(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
-    shutdown(ctx.getChannel)
+    shutdown()
   }
 
   /**
@@ -75,6 +113,6 @@ class ServiceToChannelHandler[Req, Rep](service: Service[Req, Rep], log: Logger)
     log.log(
       level, Option(cause.getMessage).getOrElse("Exception caught"), cause)
 
-    shutdown(ctx.getChannel)
+    shutdown()
   }
 }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
index df59dd91d5..2ef0ae4e74 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/ExpiringService.scala
@@ -1,5 +1,12 @@
 package com.twitter.finagle.service
 
+/**
+ * A service wrapper that expires the underlying service after a
+ * certain amount of idle time. By default, expiring calls
+ * ``.release()'' on the underlying channel, but this action is
+ * customizable.
+ */
+
 import com.twitter.util
 import com.twitter.util.{Duration, Future}
 
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala
index 3f683bc445..3eb8fb6e20 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ChannelClosingHandlerSpec.scala
@@ -4,12 +4,14 @@ import org.specs.Specification
 import org.specs.mock.Mockito
 
 import org.jboss.netty.channel.{
-  Channel, ChannelHandlerContext, ChannelStateEvent,
-  ChannelPipeline}
+  Channels, Channel, ChannelHandlerContext,
+  ChannelStateEvent, ChannelPipeline}
 
 object ChannelClosingHandlerSpec extends Specification with Mockito {
   "ChannelClosingHandler" should {
     val channel = mock[Channel]
+    val closeFuture = Channels.future(channel)
+    channel.close() returns closeFuture
     val handler = new ChannelClosingHandler
     val ctx = mock[ChannelHandlerContext]
     val e = mock[ChannelStateEvent]
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
index dc873d756d..081c103785 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/channel/ServiceToChannelHandlerSpec.scala
@@ -8,7 +8,8 @@ import org.mockito.{Matchers, ArgumentCaptor}
 
 import org.jboss.netty.channel.{
   ChannelHandlerContext, MessageEvent, Channel,
-  ChannelPipeline, DownstreamMessageEvent}
+  ChannelPipeline, DownstreamMessageEvent,
+  ChannelStateEvent, Channels}
 
 import com.twitter.util.Future
 import com.twitter.finagle.Service
@@ -23,14 +24,19 @@ object ServiceToChannelHandlerSpec extends Specification with Mockito {
     val handler = new ServiceToChannelHandler(service, log)
     val pipeline = mock[ChannelPipeline]
     val channel = mock[Channel]
+    val closeFuture = Channels.future(channel)
+    channel.close returns closeFuture
     channel.isOpen returns true
     val ctx = mock[ChannelHandlerContext]
-
     channel.getPipeline returns pipeline
     ctx.getChannel returns channel
     val e = mock[MessageEvent]
     e.getMessage returns request
 
+    // This opens the channel, so that the ClosingHandler discovers
+    // the channel.
+    handler.channelOpen(ctx, mock[ChannelStateEvent])
+
     service(Matchers.any[Foo]) answers { f => Future.value(f.asInstanceOf[Foo].fooMethod) }
 
     "when sending a valid message" in {
diff --git a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/EndToEndSpec.scala b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/EndToEndSpec.scala
index 98981e08ba..655fda8d1f 100644
--- a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/EndToEndSpec.scala
+++ b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/EndToEndSpec.scala
@@ -40,6 +40,8 @@ object EndToEndSpec extends Specification {
       .bindTo(serverAddr)
       .build(new B.Service(processor, new TBinaryProtocol.Factory()))
 
+    doAfter { server.close() }
+
     val service = ClientBuilder()
       .hosts(Seq(serverAddr))
       .codec(ThriftClientFramedCodec())
@@ -58,8 +60,6 @@ object EndToEndSpec extends Specification {
       client.add_one(1, 2)()  // don't block!
 
       client.someway()() must beNull  // don't block!
-
-      server.close()()
     }
 
     "handle wrong interface" in {
@@ -67,8 +67,6 @@ object EndToEndSpec extends Specification {
 
       client.another_method(123)() must throwA(
         new TApplicationException("Invalid method name: 'another_method'"))
-
-      server.close()()
     }
   }
 
diff --git a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ServiceEndToEndSpec.scala b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ServiceEndToEndSpec.scala
index ad48bb485f..14308e9e25 100644
--- a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ServiceEndToEndSpec.scala
+++ b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ServiceEndToEndSpec.scala
@@ -50,6 +50,8 @@ object ServiceEndToEndSpec extends Specification {
       .bindTo(addr)
       .build(sillyService)
 
+    doAfter { server.close() }
+
     "with wrapped replies" in {
       "respond to calls with ThriftReply[Call.response_type]" in {
         val client = ClientBuilder()
@@ -69,8 +71,6 @@ object ServiceEndToEndSpec extends Specification {
         result.isReturn must beTrue
         val reply = result().asInstanceOf[ThriftReply[Silly.bleep_result]]
         reply().response.success mustEqual "olleh"
-
-        server.close()()
       }
     }
   }
diff --git a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ThriftClientFinagleServerSpec.scala b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ThriftClientFinagleServerSpec.scala
index 56e3cad953..5025e1da42 100644
--- a/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ThriftClientFinagleServerSpec.scala
+++ b/finagle-thrift/src/test/scala/com/twitter/finagle/thrift/ThriftClientFinagleServerSpec.scala
@@ -40,7 +40,7 @@ object ThriftClientFinagleServerSpec extends Specification {
       .build(new B.Service(processor, new TBinaryProtocol.Factory()))
 
     doAfter {
-      server.close()()
+      server.close()
     }
 
     val (client, transport) = {
diff --git a/project/build/Project.scala b/project/build/Project.scala
index 970d5ff207..60a6c53578 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -71,7 +71,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
     val nettyRepo =
       "repository.jboss.org" at "http://repository.jboss.org/nexus/content/groups/public/"
     val netty     = "org.jboss.netty"      %  "netty"     % "3.2.3.Final"
-    val util      = "com.twitter"          %  "util"      % "1.6.1"
+    val util      = "com.twitter"          %  "util"      % "1.6.2"
 
     val mockito   = "org.mockito"             % "mockito-all" % "1.8.5" % "test" withSources()
     val specs     = "org.scala-tools.testing" % "specs_2.8.0" % "1.6.5" % "test" withSources()

From 4f04b870023d1aaaea33561672385c7f96b3615d Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Sun, 6 Feb 2011 11:37:46 -0800
Subject: [PATCH 34/43] LoadBalancedFactory: revert availability condition when
 no underlying factories are available.

---
 ...edPool.scala => LoadBalancedFactory.scala} | 28 +++++++-----
 .../LoadBalancedFactorySpec.scala             | 45 +++++++++++++++++++
 .../loadbalancer/LoadBalancedPoolSpec.scala   | 26 -----------
 3 files changed, 62 insertions(+), 37 deletions(-)
 rename finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/{LoadBalancedPool.scala => LoadBalancedFactory.scala} (51%)
 create mode 100644 finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactorySpec.scala
 delete mode 100644 finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedPoolSpec.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedPool.scala b/finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactory.scala
similarity index 51%
rename from finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedPool.scala
rename to finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactory.scala
index 39669873f8..a8182ed7ec 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedPool.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactory.scala
@@ -17,24 +17,30 @@ trait LoadBalancerStrategy[Req, Rep]
 
 // pass in strategy, etc.
 class LoadBalancedFactory[Req, Rep](
-    pools: Seq[ServiceFactory[Req, Rep]],
+    factories: Seq[ServiceFactory[Req, Rep]],
     strategy: LoadBalancerStrategy[Req, Rep])
   extends ServiceFactory[Req, Rep]
 {
-  def make() = {
-    // Make a snapshot of the pools. The passed-in seqs may not be
-    // immutable.
-    val snapshot = pools.toArray filter { _.isAvailable }
+  def make(): Future[Service[Req, Rep]] = {
+    // We first create a snapshot since the underlying seq could
+    // change.
+    val snapshot = factories.toArray
 
-    // TODO: (XXX) include *every* service if none are available.
-    
     if (snapshot.isEmpty)
-      Future.exception(new NoBrokersAvailableException)
-    else
+      return Future.exception(new NoBrokersAvailableException)
+
+    val available = snapshot.toArray filter { _.isAvailable }
+
+    // If none are available, we load balance over all of them. This
+    // is to remedy situations where the health checking becomes too
+    // pessimistic.
+    if (available.isEmpty)
       strategy(snapshot)
+    else
+      strategy(available)
   }
 
-  override def isAvailable = pools.exists(_.isAvailable)
+  override def isAvailable = factories.exists(_.isAvailable)
 
-  override def close() = pools foreach { _.close() }
+  override def close() = factories foreach { _.close() }
 }
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactorySpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactorySpec.scala
new file mode 100644
index 0000000000..069bbf404b
--- /dev/null
+++ b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedFactorySpec.scala
@@ -0,0 +1,45 @@
+package com.twitter.finagle.loadbalancer
+ 
+import org.specs.Specification
+import org.specs.mock.Mockito
+import org.mockito.Matchers
+
+import com.twitter.util.Future
+import com.twitter.finagle.{Service, ServiceFactory}
+
+object LoadBalancedFactorySpec extends Specification with Mockito {
+  "LoadBalancedFactory" should {
+    val f1 = mock[ServiceFactory[Any, Any]]
+    val f2 = mock[ServiceFactory[Any, Any]]
+    val f3 = mock[ServiceFactory[Any, Any]]
+
+    f1.isAvailable returns true
+    f2.isAvailable returns true
+    f3.isAvailable returns true
+
+    val strategy = mock[LoadBalancerStrategy[Any, Any]]
+    (strategy(Matchers.any[Seq[ServiceFactory[Any, Any]]])
+     returns Future.value(mock[Service[Any, Any]]))
+
+    val loadbalanced = new LoadBalancedFactory(Seq(f1, f2, f3), strategy)
+
+    "execute the strategy over all healthy factories" in {
+      loadbalanced.make()
+      there was one(strategy)(Seq(f1, f2, f3))
+
+      f2.isAvailable returns false
+
+      loadbalanced.make()
+      there was one(strategy)(Seq(f1, f3))
+    }
+
+    "executes strategy over all underlying factories when NONE are available" in {
+      f1.isAvailable returns false
+      f2.isAvailable returns false
+      f3.isAvailable returns false
+
+      loadbalanced.make()
+      there was one(strategy)(Seq(f1, f2, f3))
+    }
+  }
+}
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedPoolSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedPoolSpec.scala
deleted file mode 100644
index af22d6acf2..0000000000
--- a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LoadBalancedPoolSpec.scala
+++ /dev/null
@@ -1,26 +0,0 @@
-package com.twitter.finagle.loadbalancer
- 
-// check availability filtering!
-
-// TODO:
-//    "fall back to no filtering if everything is marked dead" in {
-//      Time.withCurrentTimeFrozen { timeControl =>
-//        val failureAccrual = new FailureAccrualStrategy[Int, Int](underlying, 2, 10.seconds)
-//         
-//        underlying.dispatch(123, services) returns Some(service, Future.exception(new Exception))
-//        failureAccrual.dispatch(123, services) must beSomething
-//        failureAccrual.dispatch(123, services) must beSomething
-//        there were two(underlying).dispatch(123, services)
-//         
-//        // The one should have failed now. Make the other one fail as well.
-//        (underlying.dispatch(123, Seq(service1))
-//           returns Some(service1, Future.exception(new Exception)))
-//        failureAccrual.dispatch(123, services) must beSomething
-//        failureAccrual.dispatch(123, services) must beSomething
-//        there were two(underlying).dispatch(123, services)
-//        there were two(underlying).dispatch(123, Seq(service1))
-//         
-//        // They're both failed, so we should revert now.
-//        failureAccrual.dispatch(123, services) must beSomething
-//        there were three(underlying).dispatch(123, services)
-//      }

From 2c919e6adea85289c582e79d970cfbbc03eb8548 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Sun, 6 Feb 2011 12:02:39 -0800
Subject: [PATCH 35/43] introduce reference-counted timers for proper daemon
 thread shutdown.

---
 .../com/twitter/finagle/builder/ClientBuilder.scala    |  9 ++++++++-
 .../com/twitter/finagle/builder/ServerBuilder.scala    |  2 ++
 .../com/twitter/finagle/service/TimeoutFilter.scala    |  3 ++-
 .../finagle/stats/JavaLoggerStatsReceiver.scala        |  3 ++-
 .../twitter/finagle/util/ReferenceCountedTimer.scala   |  4 ++++
 .../scala/com/twitter/finagle/util/RichFuture.scala    |  3 ++-
 .../main/scala/com/twitter/finagle/util/Timer.scala    | 10 +++++++---
 7 files changed, 27 insertions(+), 7 deletions(-)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/util/ReferenceCountedTimer.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
index d496e96e26..a16bbd4a7d 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ClientBuilder.scala
@@ -255,6 +255,8 @@ case class ClientBuilder[Req, Rep](
     if (!_protocol.isDefined)
       throw new IncompleteSpecification("No protocol was specified")
 
+    Timer.default.acquire()
+
     val cluster  = _cluster.get
     val protocol = _protocol.get
 
@@ -288,7 +290,12 @@ case class ClientBuilder[Req, Rep](
       factory
     }
 
-    new LoadBalancedFactory(hostFactories, new LeastQueuedStrategy[Req, Rep])
+    new LoadBalancedFactory(hostFactories, new LeastQueuedStrategy[Req, Rep]) {
+      override def close() = {
+        super.close()
+        Timer.default.stop()
+      }
+    }
   }
 
   def build(): Service[Req, Rep] = {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
index abec7a36c8..efc51eb4ff 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/ServerBuilder.scala
@@ -230,6 +230,7 @@ case class ServerBuilder[Req, Rep](
     })
 
     val serverChannel = bs.bind(_bindTo.get)
+    Timer.default.acquire()
     new Server {
       def close(timeout: Duration = Duration.MaxValue) = {
         // According to NETTY-256, the following sequence of operations
@@ -265,6 +266,7 @@ case class ServerBuilder[Req, Rep](
         channels.synchronized { channels foreach { _.close() } }
 
         bs.releaseExternalResources()
+        Timer.default.stop()
       }
     }
   }
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
index ea0dc03870..6e6b5b8321 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/TimeoutFilter.scala
@@ -1,5 +1,6 @@
 package com.twitter.finagle.service
 
+import com.twitter.util
 import com.twitter.util.{Future, Duration, Throw}
 
 import com.twitter.finagle.TimedoutRequestException
@@ -11,7 +12,7 @@ import com.twitter.finagle.{Filter, Service}
  * A filter to apply a global timeout to the request. This allows,
  * e.g., for a server to apply a global request timeout.
  */
-class TimeoutFilter[Req, Rep](timeout: Duration, timer: Timer = Timer.default)
+class TimeoutFilter[Req, Rep](timeout: Duration, timer: util.Timer = Timer.default)
   extends Filter[Req, Rep, Req, Rep]
 {
   def apply(request: Req, service: Service[Req, Rep]): Future[Rep] = {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/stats/JavaLoggerStatsReceiver.scala b/finagle-core/src/main/scala/com/twitter/finagle/stats/JavaLoggerStatsReceiver.scala
index 9dd32bea08..d43d0c3da5 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/stats/JavaLoggerStatsReceiver.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/stats/JavaLoggerStatsReceiver.scala
@@ -2,10 +2,11 @@ package com.twitter.finagle.stats
 
 import java.util.logging.Logger
 import com.twitter.conversions.time._
+import com.twitter.util
 import com.twitter.finagle.util.Conversions._
 import com.twitter.finagle.util.Timer
 
-class JavaLoggerStatsReceiver(logger: Logger, timer: Timer) extends StatsReceiver {
+class JavaLoggerStatsReceiver(logger: Logger, timer: util.Timer) extends StatsReceiver {
   def this(logger: Logger) = this(logger, Timer.default)
 
   def gauge(description: (String, String)*) = new Gauge {
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/ReferenceCountedTimer.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/ReferenceCountedTimer.scala
new file mode 100644
index 0000000000..453de38f81
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/ReferenceCountedTimer.scala
@@ -0,0 +1,4 @@
+package com.twitter.finagle.util
+
+import org.jboss.netty.util.{Timer => NettyTimer}
+
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/RichFuture.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/RichFuture.scala
index 1872e0d3e8..1b0db10e20 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/util/RichFuture.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/RichFuture.scala
@@ -1,11 +1,12 @@
 package com.twitter.finagle.util
 
+import com.twitter.util
 import com.twitter.util.{Duration, Future, Promise, Try}
 
 import Conversions._
 
 class RichFuture[A](self: Future[A]) {
-  def timeout(timer: Timer, howlong: Duration)(orElse: => Try[A]) = {
+  def timeout(timer: util.Timer, howlong: Duration)(orElse: => Try[A]) = {
     val promise = new Promise[A]
     val timeout = timer.schedule(howlong.fromNow) { promise.updateIfEmpty(orElse) }
     self respond { r =>
diff --git a/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala b/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
index cb6472ee03..d89edeb539 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/util/Timer.scala
@@ -1,12 +1,16 @@
 package com.twitter.finagle.util
 
 import java.util.concurrent.TimeUnit
-import com.twitter.util.{Time, Duration, TimerTask}
+import com.twitter.util.{Time, Duration, TimerTask, ReferenceCountedTimer}
 import org.jboss.netty.util.{HashedWheelTimer, Timeout}
 
 object Timer {
-  val defaultNettyTimer = new HashedWheelTimer(10, TimeUnit.MILLISECONDS)
-  implicit lazy val default = new Timer(defaultNettyTimer)
+  // This timer should only be used inside the context of finagle,
+  // since it requires explicit reference count management. (Via the
+  // builder routines.)
+  implicit val default =
+    new ReferenceCountedTimer(() =>
+      new Timer(new HashedWheelTimer(10, TimeUnit.MILLISECONDS)))
 }
 
 class Timer(underlying: org.jboss.netty.util.Timer) extends com.twitter.util.Timer 

From 7987c637f617419b6502732feb0fa968747a9c84 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Sun, 6 Feb 2011 14:58:05 -0800
Subject: [PATCH 36/43] LeftFoldUpstreamHandler: an upstream channel handler
 that is a left-fold over all channel events

---
 .../channel/LeftFoldUpstreamHandler.scala     | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/channel/LeftFoldUpstreamHandler.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/channel/LeftFoldUpstreamHandler.scala b/finagle-core/src/main/scala/com/twitter/finagle/channel/LeftFoldUpstreamHandler.scala
new file mode 100644
index 0000000000..0acd0f32bf
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/channel/LeftFoldUpstreamHandler.scala
@@ -0,0 +1,163 @@
+package com.twitter.finagle.channel
+
+/**
+ * Introduces a "foldable" channel handler for easy state machine
+ * management. For certain use cases, these both simplify state
+ * machines and enhance composability.
+ */
+
+import org.jboss.netty.channel._
+
+class LeftFoldUpstreamHandler {
+  def channelHandler = new LeftFoldHandlerToChannelHandler(this)
+
+  def channelBound(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def channelClosed(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def channelConnected(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def channelDisconnected(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def channelInterestChanged(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def channelOpen(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+  
+  def channelUnbound(
+    ctx: ChannelHandlerContext,
+    e: ChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def childChannelClosed(
+    ctx: ChannelHandlerContext,
+    e: ChildChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def childChannelOpen(
+    ctx: ChannelHandlerContext,
+    e: ChildChannelStateEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def exceptionCaught(
+    ctx: ChannelHandlerContext,
+    e: ExceptionEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def messageReceived(
+    ctx: ChannelHandlerContext,
+    e: MessageEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+
+  def writeComplete(
+    ctx: ChannelHandlerContext,
+    e: WriteCompletionEvent): LeftFoldUpstreamHandler =
+  {
+    ctx.sendUpstream(e)
+    this
+  }
+}
+
+private[channel] class LeftFoldHandlerToChannelHandler(initial: LeftFoldUpstreamHandler)
+  extends SimpleChannelUpstreamHandler
+{
+  private[this] var state = initial
+
+  override def channelBound(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelBound(ctx, e)
+  }
+
+  override def channelClosed(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelClosed(ctx, e)
+  }
+
+  override def channelConnected(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelConnected(ctx, e)
+  }
+
+  override def channelDisconnected(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelDisconnected(ctx, e)
+  }
+
+  override def channelInterestChanged(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelInterestChanged(ctx, e)
+  }
+
+  override def channelOpen(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelOpen(ctx, e)
+  }
+
+  override def channelUnbound(ctx: ChannelHandlerContext, e: ChannelStateEvent) {
+    state = state.channelUnbound(ctx, e)
+  }
+
+  override def childChannelClosed(ctx: ChannelHandlerContext, e: ChildChannelStateEvent) {
+    state = state.childChannelClosed(ctx, e)
+  }
+
+  override def childChannelOpen(ctx: ChannelHandlerContext, e: ChildChannelStateEvent) {
+    state = state.childChannelOpen(ctx, e)
+  }
+
+  override def exceptionCaught(ctx: ChannelHandlerContext, e: ExceptionEvent) {
+    state = state.exceptionCaught(ctx, e)
+  }
+
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) {
+    state = state.messageReceived(ctx, e)
+  }
+
+  override def writeComplete(ctx: ChannelHandlerContext, e: WriteCompletionEvent) {
+    state = state.writeComplete(ctx, e)
+  }
+}

From 0ce06ee87fac111df57e8c6a7a1dade058cfe4d4 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Sun, 6 Feb 2011 15:00:05 -0800
Subject: [PATCH 37/43] a handler for aggregating HTTP requests (ie. POSTS with
 100/continue)

---
 .../finagle/http/AggregateHttpRequest.scala   | 104 ++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala
new file mode 100644
index 0000000000..9762e48760
--- /dev/null
+++ b/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala
@@ -0,0 +1,104 @@
+package com.twitter.finagle.http
+
+/**
+ * Aggregate HTTP chunked HTTP requests (ie. POSTs with 100-continue)
+ */
+
+import scala.collection.JavaConversions._
+
+import java.nio.ByteOrder
+
+import org.jboss.netty.channel.{
+  MessageEvent, Channels, SimpleChannelUpstreamHandler,
+  ChannelHandlerContext}
+import org.jboss.netty.handler.codec.http.{
+  HttpHeaders, HttpRequest, HttpResponse, HttpChunk,
+  DefaultHttpResponse, HttpVersion, HttpResponseStatus}
+import org.jboss.netty.buffer.{
+  ChannelBuffer, ChannelBuffers,
+  CompositeChannelBuffer}
+
+import com.twitter.finagle.util.Conversions._
+import com.twitter.finagle.channel.LeftFoldUpstreamHandler
+
+object OneHundredContinueResponse
+  extends DefaultHttpResponse(
+    HttpVersion.HTTP_1_1,
+    HttpResponseStatus.CONTINUE)
+
+class HttpFailure(ctx: ChannelHandlerContext, status: HttpResponseStatus)
+  extends LeftFoldUpstreamHandler
+{
+  { // TODO: always transition to an untenable state
+    val response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status)
+    val future = Channels.future(ctx.getChannel)
+    Channels.write(ctx, future, response, ctx.getChannel.getRemoteAddress)    
+    future onSuccessOrFailure { ctx.getChannel.close() }
+  }
+
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) =
+    this  // (swallow the message)
+}
+
+case class AggregateHttpChunks(
+    whenDone: LeftFoldUpstreamHandler,
+    request: HttpRequest,
+    maxSize: Int,
+    bytesSoFar: Int = 0,
+    chunks: List[ChannelBuffer] = Nil)
+  extends LeftFoldUpstreamHandler
+{
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) =
+    e.getMessage match {
+      case chunk: HttpChunk =>
+        val chunkBuffer = chunk.getContent
+        val chunkBufferSize = chunkBuffer.readableBytes
+        if (bytesSoFar + chunkBufferSize > maxSize) {
+          new HttpFailure(ctx, HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE)
+        } else if (chunk.isLast) {
+          // Remove traces of request chunking.
+          val encodings = request.getHeaders(HttpHeaders.Names.TRANSFER_ENCODING)
+          encodings.remove(HttpHeaders.Values.CHUNKED)
+          if (encodings.isEmpty())
+            request.removeHeader(HttpHeaders.Names.TRANSFER_ENCODING);
+          request.setChunked(false)
+
+          // Set the content
+          val compositeBuffer =
+            new CompositeChannelBuffer(ByteOrder.BIG_ENDIAN, (chunkBuffer :: chunks).reverse)
+          request.setContent(compositeBuffer)
+
+          // And let it on its way.
+          Channels.fireMessageReceived(ctx, request)
+
+          // Transition back to initial state.
+          whenDone
+        } else {
+          copy(bytesSoFar = bytesSoFar + chunkBufferSize,
+               chunks     = chunkBuffer :: chunks)
+        }
+
+      case _ =>
+        new HttpFailure(ctx, HttpResponseStatus.BAD_REQUEST)
+    }
+}
+
+class AggregateHttpRequest(maxBufferSize: Int)
+  extends LeftFoldUpstreamHandler
+{
+  override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) =
+    e.getMessage match {
+      case request: HttpRequest if HttpHeaders.is100ContinueExpected(request) =>
+        if (HttpHeaders.getContentLength(request, -1L) > maxBufferSize) {
+          new HttpFailure(ctx, HttpResponseStatus.EXPECTATION_FAILED)
+        } else {
+          val future = Channels.future(ctx.getChannel)
+          Channels.write(ctx, future, OneHundredContinueResponse, e.getRemoteAddress)
+          request.removeHeader(HttpHeaders.Names.EXPECT)
+          new AggregateHttpChunks(this, request, maxBufferSize)
+        }
+
+      case _ =>
+        super.messageReceived(ctx, e)
+    }
+}

From b4832b1f378764550c42dee37dd2324f3494c7dc Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Sun, 6 Feb 2011 15:00:49 -0800
Subject: [PATCH 38/43] use request aggregation in the HTTP codec.

---
 .../src/main/scala/com/twitter/finagle/builder/Http.scala     | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
index 9d2fe73366..77bbc44821 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/builder/Http.scala
@@ -34,6 +34,10 @@ class Http(compressionLevel: Int = 0) extends Codec[HttpRequest, HttpResponse] {
             new HttpContentCompressor(compressionLevel))
         }
 
+        pipeline.addLast(
+          "httpRequestAggregator",
+          (new AggregateHttpRequest(10 << 20)).channelHandler)
+
         pipeline.addLast(
           "connectionLifecycleManager",
           new ServerConnectionManager)

From 3eda1e19cc766366a08f80f4aadc8c252201a7f2 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Mon, 7 Feb 2011 10:16:24 -0800
Subject: [PATCH 39/43] refcount Timers in unit & stress tests.

---
 .../scala/com/twitter/finagle/service/TimeoutFilterSpec.scala | 3 +++
 .../test/scala/com/twitter/finagle/util/RichFutureSpec.scala  | 2 ++
 .../src/test/scala/com/twitter/finagle/util/TimerSpec.scala   | 4 +++-
 .../scala/com/twitter/finagle/stress/EmbeddedServer.scala     | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
index 03fdea3029..a2b89d1bd3 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/service/TimeoutFilterSpec.scala
@@ -11,6 +11,9 @@ import com.twitter.finagle.Service
 object TimeoutFilterSpec extends Specification with Mockito {
   "TimeoutFilter" should {
     val timer = Timer.default
+    timer.acquire()
+    doAfter { timer.stop() }
+
     val promise = new Promise[String]
     val service = new Service[String, String] {
       def apply(request: String) = promise
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/util/RichFutureSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/util/RichFutureSpec.scala
index 47ee5683fd..b88789a901 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/util/RichFutureSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/util/RichFutureSpec.scala
@@ -11,6 +11,8 @@ object RichFutureSpec extends Specification with Mockito {
   "RichFuture" should {
     "timeout" in {
       val timer = Timer.default
+      timer.acquire()
+      doAfter { timer.stop() }
       val alternative = Try(2)
       val richFuture = new Promise[Int].timeout(timer, 1.second) {
         alternative
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/util/TimerSpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/util/TimerSpec.scala
index dfa9b2cec4..174fdfce67 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/util/TimerSpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/util/TimerSpec.scala
@@ -9,6 +9,8 @@ import com.twitter.conversions.time._
 object TimerSpec extends Specification with Mockito {
   "Timer" should {
     val timer = Timer.default
+    timer.acquire()
+    doAfter { timer.stop() }
 
     val start = Time.now
     var end = Time.now
@@ -28,4 +30,4 @@ object TimerSpec extends Specification with Mockito {
       latch.await(2.seconds) must beFalse
     }
   }
-}
\ No newline at end of file
+}
diff --git a/finagle-stress/src/main/scala/com/twitter/finagle/stress/EmbeddedServer.scala b/finagle-stress/src/main/scala/com/twitter/finagle/stress/EmbeddedServer.scala
index fdb39ae41b..f647fa2923 100644
--- a/finagle-stress/src/main/scala/com/twitter/finagle/stress/EmbeddedServer.scala
+++ b/finagle-stress/src/main/scala/com/twitter/finagle/stress/EmbeddedServer.scala
@@ -22,6 +22,7 @@ import com.twitter.finagle.util.Timer
 object EmbeddedServer {
   def apply() = new EmbeddedServer(RandomSocket())
   val timer = Timer.default
+  timer.acquire()
 }
 
 class EmbeddedServer(val addr: SocketAddress) {

From d9dbe8771020ceb3020245568d4e9086491c5d72 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Mon, 7 Feb 2011 10:16:41 -0800
Subject: [PATCH 40/43] util 1.6.3

---
 project/build/Project.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/project/build/Project.scala b/project/build/Project.scala
index 60a6c53578..d5cb813fa0 100644
--- a/project/build/Project.scala
+++ b/project/build/Project.scala
@@ -71,7 +71,7 @@ class Project(info: ProjectInfo) extends StandardParentProject(info)
     val nettyRepo =
       "repository.jboss.org" at "http://repository.jboss.org/nexus/content/groups/public/"
     val netty     = "org.jboss.netty"      %  "netty"     % "3.2.3.Final"
-    val util      = "com.twitter"          %  "util"      % "1.6.2"
+    val util      = "com.twitter"          %  "util"      % "1.6.3"
 
     val mockito   = "org.mockito"             % "mockito-all" % "1.8.5" % "test" withSources()
     val specs     = "org.scala-tools.testing" % "specs_2.8.0" % "1.6.5" % "test" withSources()

From 26baedb3626130f218692253782d68c42ec52970 Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Mon, 7 Feb 2011 10:16:54 -0800
Subject: [PATCH 41/43] clean up AggregateHttpRequest a bit.

---
 .../finagle/http/AggregateHttpRequest.scala   | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala b/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala
index 9762e48760..6dfc159c54 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/http/AggregateHttpRequest.scala
@@ -9,11 +9,14 @@ import scala.collection.JavaConversions._
 import java.nio.ByteOrder
 
 import org.jboss.netty.channel.{
-  MessageEvent, Channels, SimpleChannelUpstreamHandler,
+  MessageEvent, Channels,
+  SimpleChannelUpstreamHandler,
   ChannelHandlerContext}
 import org.jboss.netty.handler.codec.http.{
-  HttpHeaders, HttpRequest, HttpResponse, HttpChunk,
-  DefaultHttpResponse, HttpVersion, HttpResponseStatus}
+  HttpHeaders, HttpRequest,
+  HttpResponse, HttpChunk,
+  DefaultHttpResponse,
+  HttpVersion, HttpResponseStatus}
 import org.jboss.netty.buffer.{
   ChannelBuffer, ChannelBuffers,
   CompositeChannelBuffer}
@@ -29,7 +32,7 @@ object OneHundredContinueResponse
 class HttpFailure(ctx: ChannelHandlerContext, status: HttpResponseStatus)
   extends LeftFoldUpstreamHandler
 {
-  { // TODO: always transition to an untenable state
+  {
     val response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, status)
     val future = Channels.future(ctx.getChannel)
     Channels.write(ctx, future, response, ctx.getChannel.getRemoteAddress)    
@@ -43,9 +46,8 @@ class HttpFailure(ctx: ChannelHandlerContext, status: HttpResponseStatus)
 case class AggregateHttpChunks(
     whenDone: LeftFoldUpstreamHandler,
     request: HttpRequest,
-    maxSize: Int,
-    bytesSoFar: Int = 0,
-    chunks: List[ChannelBuffer] = Nil)
+    bufferBudget: Int,
+    buffer: ChannelBuffer = ChannelBuffers.EMPTY_BUFFER)
   extends LeftFoldUpstreamHandler
 {
   override def messageReceived(ctx: ChannelHandlerContext, e: MessageEvent) =
@@ -53,20 +55,20 @@ case class AggregateHttpChunks(
       case chunk: HttpChunk =>
         val chunkBuffer = chunk.getContent
         val chunkBufferSize = chunkBuffer.readableBytes
-        if (bytesSoFar + chunkBufferSize > maxSize) {
+
+        if (chunkBufferSize > bufferBudget) {
           new HttpFailure(ctx, HttpResponseStatus.REQUEST_ENTITY_TOO_LARGE)
         } else if (chunk.isLast) {
           // Remove traces of request chunking.
           val encodings = request.getHeaders(HttpHeaders.Names.TRANSFER_ENCODING)
           encodings.remove(HttpHeaders.Values.CHUNKED)
-          if (encodings.isEmpty())
-            request.removeHeader(HttpHeaders.Names.TRANSFER_ENCODING);
+          if (encodings.isEmpty)
+            request.removeHeader(HttpHeaders.Names.TRANSFER_ENCODING)
+
           request.setChunked(false)
 
           // Set the content
-          val compositeBuffer =
-            new CompositeChannelBuffer(ByteOrder.BIG_ENDIAN, (chunkBuffer :: chunks).reverse)
-          request.setContent(compositeBuffer)
+          request.setContent(ChannelBuffers.wrappedBuffer(buffer, chunkBuffer))
 
           // And let it on its way.
           Channels.fireMessageReceived(ctx, request)
@@ -74,8 +76,8 @@ case class AggregateHttpChunks(
           // Transition back to initial state.
           whenDone
         } else {
-          copy(bytesSoFar = bytesSoFar + chunkBufferSize,
-               chunks     = chunkBuffer :: chunks)
+          copy(bufferBudget = bufferBudget - chunkBufferSize,
+               buffer       = ChannelBuffers.wrappedBuffer(buffer, chunkBuffer))
         }
 
       case _ =>
@@ -94,6 +96,9 @@ class AggregateHttpRequest(maxBufferSize: Int)
         } else {
           val future = Channels.future(ctx.getChannel)
           Channels.write(ctx, future, OneHundredContinueResponse, e.getRemoteAddress)
+
+          // Remove the the ``Expect:'' header and continue with
+          // collecting chunks.
           request.removeHeader(HttpHeaders.Names.EXPECT)
           new AggregateHttpChunks(this, request, maxBufferSize)
         }

From 47a36800a49e601c994165fbff69f3c44d89a98b Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Mon, 7 Feb 2011 10:38:07 -0800
Subject: [PATCH 42/43] propagate isAvailable in SingletonFactory.

---
 .../scala/com/twitter/finagle/service/SingletonFactory.scala    | 2 ++
 .../twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
index 8838392e51..3e2490592d 100644
--- a/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
+++ b/finagle-core/src/main/scala/com/twitter/finagle/service/SingletonFactory.scala
@@ -19,4 +19,6 @@ class SingletonFactory[Req, Rep](service: Service[Req, Rep])
   }
 
   def close() = latch.await { service.release() }
+
+  override def isAvailable = service.isAvailable
 }
diff --git a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
index b4e6f70212..4565cd99b6 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
@@ -16,6 +16,8 @@ object LeastLoadedStrategySpec extends Specification with Mockito {
       numInvocations += 1
       numInvocations
     }
+
+    override def isAvailable = true
   }
 
   "LeastLoadedStrategy" should {

From 9ddc84623494e2e070b5cd3bb070a849200ed5df Mon Sep 17 00:00:00 2001
From: "marius a. eriksen" <marius@twitter.com>
Date: Mon, 7 Feb 2011 10:47:20 -0800
Subject: [PATCH 43/43] fix one more timer reference.

---
 .../finagle/loadbalancer/LeastLoadedStrategySpec.scala       | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
index 4565cd99b6..6eb48d6e78 100644
--- a/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
+++ b/finagle-core/src/test/scala/com/twitter/finagle/loadbalancer/LeastLoadedStrategySpec.scala
@@ -6,6 +6,7 @@ import org.specs.mock.Mockito
 import com.twitter.util.{Future, Time}
 
 import com.twitter.finagle.Service
+import com.twitter.finagle.util.Timer
 import com.twitter.finagle.service.SingletonFactory
 
 object LeastLoadedStrategySpec extends Specification with Mockito {
@@ -20,6 +21,10 @@ object LeastLoadedStrategySpec extends Specification with Mockito {
     override def isAvailable = true
   }
 
+  // FIXME: This is non-obvious & fragile.
+  Timer.default.acquire()
+  doBefore { Timer.default.stop() }
+
   "LeastLoadedStrategy" should {
 
     "distribute load evently" in {