Fix frame leak due to undrained send buffer (#860)

* Fix frame leak due to undrained send buffer

Follow up to #845

Currently in `Connection.writeFrames()`, we return immediately
upon a connection error. However, at that point there may still
be frames stuck in the send buffer (`Connection.sendCh`) which
haven't been sent due to the connection issue. Leaving it in
such a state results in a frame leak since they will never be
released.

Change `writeFrames()` to always drain the send buffer upon return
to fix this behavior.

connection.go

@@ -740,6 +740,15 @@ func (c *Connection) handleFrameNoRelay(frame *Frame) bool {
 // writeFrames is the main loop that pulls frames from the send channel and
 // writes them to the connection.
 func (c *Connection) writeFrames(_ uint32) {
+	defer func() {
+		<-c.stopCh
+		// Drain and release any remaining frames in sendCh for best-effort
+		// reduction in leaked frames
+		for len(c.sendCh) > 0 {
+			c.opts.FramePool.Release(<-c.sendCh)
+		}
+	}()
+
 	for {
 		select {
 		case f := <-c.sendCh:

relay_test.go

@@ -339,7 +339,9 @@ func TestTimeoutCallsThenClose(t *testing.T) {
 	// Test needs at least 2 CPUs to trigger race conditions.
 	defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2))
 
-	opts := serviceNameOpts("s1").SetRelayOnly().DisableLogVerification()
+	opts := serviceNameOpts("s1").
+		SetRelayOnly().
+		DisableLogVerification()
 	testutils.WithTestServer(t, opts, func(t testing.TB, ts *testutils.TestServer) {
 		s1 := ts.Server()
 		s2 := ts.NewServer(serviceNameOpts("s2").DisableLogVerification())
@@ -369,14 +371,14 @@ func TestTimeoutCallsThenClose(t *testing.T) {
 }
 
 func TestLargeTimeoutsAreClamped(t *testing.T) {
-	// TODO: enable frame pool checks
 	const (
 		clampTTL = time.Millisecond
 		longTTL  = time.Minute
 	)
 
 	opts := serviceNameOpts("echo-service").
 		SetRelayOnly().
+		SetCheckFramePooling().
 		SetRelayMaxTimeout(clampTTL).
 		DisableLogVerification() // handler returns after deadline
 
@@ -910,10 +912,10 @@ func TestRelayStalledConnection(t *testing.T) {
 // Test that a stalled connection to the client does not cause stuck calls
 // See https://github.com/uber/tchannel-go/issues/700 for more info.
 func TestRelayStalledClientConnection(t *testing.T) {
-	// TODO: enable framepool checks
 	// This needs to be large enough to fill up the client TCP buffer.
 	const _calls = 100
 
+	// TODO: enable framepool checks
 	opts := testutils.NewOpts().
 		// Expect errors from dropped frames.
 		AddLogFilter("Dropping call due to slow connection.", _calls).