From 6c2bbc344561775025a0ff47fbbaeaa9e15b424b Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 6 Jan 2022 12:19:45 -0800
Subject: [PATCH] fix: ensure that pings don't get stuck behind writes

This sends pings/pongs first before processing messages.

This ensures that:

1. RTT times are accurate.
2. We don't end up thinking the connection is dead because a ping keeps
   getting stuck behind a message.

I tried lowering the max message size to 4KiB, but that broke tests due
to a perf regression. Although I'm not sure how "real" that perf
regression is.
---
 session.go | 67 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/session.go b/session.go
index 1027715..f7e2453 100644
--- a/session.go
+++ b/session.go
@@ -501,11 +501,9 @@ func (s *Session) sendLoop() error {
 		default:
 		}
 
-		// Flushes at least once every 100 microseconds unless we're
-		// constantly writing.
 		var buf []byte
+		// Make sure to send any pings & pongs first so they don't get stuck behind writes.
 		select {
-		case buf = <-s.sendCh:
 		case pingID := <-s.pingCh:
 			buf = pool.Get(headerSize)
 			hdr := encode(typePing, flagSYN, 0, pingID)
@@ -514,31 +512,44 @@ func (s *Session) sendLoop() error {
 			buf = pool.Get(headerSize)
 			hdr := encode(typePing, flagACK, 0, pingID)
 			copy(buf, hdr[:])
-		case <-s.shutdownCh:
-			return nil
-			//default:
-			//	select {
-			//	case buf = <-s.sendCh:
-			//	case <-s.shutdownCh:
-			//		return nil
-			//	case <-writeTimeoutCh:
-			//		if err := writer.Flush(); err != nil {
-			//			if os.IsTimeout(err) {
-			//				err = ErrConnectionWriteTimeout
-			//			}
-			//			return err
-			//		}
-
-			//		select {
-			//		case buf = <-s.sendCh:
-			//		case <-s.shutdownCh:
-			//			return nil
-			//		}
-
-			//		if writeTimeout != nil {
-			//			writeTimeout.Reset(s.config.WriteCoalesceDelay)
-			//		}
-			//	}
+		default:
+			// Then send normal data.
+			select {
+			case buf = <-s.sendCh:
+			case pingID := <-s.pingCh:
+				buf = pool.Get(headerSize)
+				hdr := encode(typePing, flagSYN, 0, pingID)
+				copy(buf, hdr[:])
+			case pingID := <-s.pongCh:
+				buf = pool.Get(headerSize)
+				hdr := encode(typePing, flagACK, 0, pingID)
+				copy(buf, hdr[:])
+			case <-s.shutdownCh:
+				return nil
+				//default:
+				//	select {
+				//	case buf = <-s.sendCh:
+				//	case <-s.shutdownCh:
+				//		return nil
+				//	case <-writeTimeoutCh:
+				//		if err := writer.Flush(); err != nil {
+				//			if os.IsTimeout(err) {
+				//				err = ErrConnectionWriteTimeout
+				//			}
+				//			return err
+				//		}
+
+				//		select {
+				//		case buf = <-s.sendCh:
+				//		case <-s.shutdownCh:
+				//			return nil
+				//		}
+
+				//		if writeTimeout != nil {
+				//			writeTimeout.Reset(s.config.WriteCoalesceDelay)
+				//		}
+				//	}
+			}
 		}
 
 		if err := extendWriteDeadline(); err != nil {