Don't start the reader thread until after the connection preface.

This is slightly more work than ideal because our tests don't bother with the
connection preface. That makes the tests both simpler, and further from reality.
The workaround is a package-private method to keep the tests working as they
are currently.

Closes square#2469

mockwebserver/src/main/java/okhttp3/internal/framed/FramedServer.java

@@ -70,7 +70,7 @@ private void run() throws Exception {
             .protocol(protocol)
             .listener(this)
             .build();
-        framedConnection.sendConnectionPreface();
+        framedConnection.start();
       } catch (IOException e) {
         logger.log(Level.INFO, "FramedServer connection failure: " + e);
         Util.closeQuietly(socket);

mockwebserver/src/main/java/okhttp3/mockwebserver/MockWebServer.java

@@ -449,6 +449,7 @@ public void processConnection() throws Exception {
               .protocol(protocol)
               .listener(framedSocketListener)
               .build();
+          framedConnection.start();
           openFramedConnections.add(framedConnection);
           openClientSockets.remove(socket);
           return;

okhttp-tests/src/test/java/okhttp3/internal/framed/Http2ConnectionTest.java

@@ -376,7 +376,9 @@ private Buffer data(int byteCount) {
 
     // play it back
     FramedConnection connection = connectionBuilder(peer, HTTP_2)
-        .pushObserver(observer).build();
+        .pushObserver(observer)
+        .build();
+    connection.start(false);
     FramedStream client = connection.newStream(headerEntries("b", "banana"), false, true);
     assertEquals(-1, client.getSource().read(new Buffer(), 1));
 
@@ -399,6 +401,7 @@ private Buffer data(int byteCount) {
 
     // play it back
     FramedConnection connection = connectionBuilder(peer, HTTP_2).build();
+    connection.start(false);
     connection.newStream(headerEntries("b", "banana"), false, true);
 
     // verify the peer received what was expected
@@ -423,8 +426,10 @@ private Buffer data(int byteCount) {
     peer.play();
 
     // play it back
-    connectionBuilder(peer, HTTP_2)
-        .pushObserver(PushObserver.CANCEL).build();
+    FramedConnection connection = connectionBuilder(peer, HTTP_2)
+        .pushObserver(PushObserver.CANCEL)
+        .build();
+    connection.start(false);
 
     // verify the peer received what was expected
     MockSpdyPeer.InFrame rstStream = peer.takeFrame();
@@ -452,6 +457,7 @@ private Buffer data(int byteCount) {
         .pushObserver(IGNORE)
         .protocol(HTTP_2.getProtocol())
         .build();
+    connection.start(false);
     socket.shutdownOutput();
     try {
       connection.newStream(headerEntries("a", longString), false, true);
@@ -488,7 +494,9 @@ private FramedConnection sendHttp2SettingsAndCheckForAck(boolean client, Setting
   }
 
   private FramedConnection connection(MockSpdyPeer peer, Variant variant) throws IOException {
-    return connectionBuilder(peer, variant).build();
+    FramedConnection connection = connectionBuilder(peer, variant).build();
+    connection.start(false);
+    return connection;
   }
 
   private FramedConnection.Builder connectionBuilder(MockSpdyPeer peer, Variant variant)

okhttp-tests/src/test/java/okhttp3/internal/framed/Spdy3ConnectionTest.java

@@ -162,10 +162,11 @@ public final class Spdy3ConnectionTest {
         stream.reply(headerEntries("b", "banana"), true);
       }
     };
-    new FramedConnection.Builder(true)
+    FramedConnection connection = new FramedConnection.Builder(true)
         .socket(peer.openSocket())
         .listener(handler)
         .build();
+    connection.start(false);
 
     // verify the peer received what was expected
     MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -192,7 +193,10 @@ public final class Spdy3ConnectionTest {
       }
     };
 
-    connectionBuilder(peer, SPDY3).listener(listener).build();
+    FramedConnection connection = connectionBuilder(peer, SPDY3)
+        .listener(listener)
+        .build();
+    connection.start(false);
 
     // verify the peer received what was expected
     MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -282,6 +286,7 @@ public final class Spdy3ConnectionTest {
     FramedConnection connection = connectionBuilder(peer, SPDY3)
         .listener(listener)
         .build();
+    connection.start(false);
 
     peer.takeFrame(); // Guarantees that the peer Settings frame has been processed.
     synchronized (connection) {
@@ -639,10 +644,11 @@ public final class Spdy3ConnectionTest {
         stream.reply(headerEntries("c", "cola"), true);
       }
     };
-    new FramedConnection.Builder(true)
+    FramedConnection connection = new FramedConnection.Builder(true)
         .socket(peer.openSocket())
         .listener(listener)
         .build();
+    connection.start(false);
 
     // verify the peer received what was expected
     MockSpdyPeer.InFrame reply = peer.takeFrame();
@@ -1340,6 +1346,7 @@ private void headerBlockHasTrailingCompressedBytes(String frame, int length) thr
         .socket(socket)
         .protocol(SPDY3.getProtocol())
         .build();
+    connection.start(false);
     socket.shutdownOutput();
     try {
       connection.newStream(headerEntries("a", longString), false, true);
@@ -1360,7 +1367,9 @@ private byte[] randomBytes(int length) {
   }
 
   private FramedConnection connection(MockSpdyPeer peer, Variant variant) throws IOException {
-    return connectionBuilder(peer, variant).build();
+    FramedConnection connection = connectionBuilder(peer, variant).build();
+    connection.start(false);
+    return connection;
   }
 
   private FramedConnection.Builder connectionBuilder(MockSpdyPeer peer, Variant variant)

okhttp/src/main/java/okhttp3/internal/framed/FramedConnection.java

@@ -170,7 +170,6 @@ private FramedConnection(Builder builder) throws IOException {
     frameWriter = variant.newWriter(builder.sink, client);
 
     readerRunnable = new Reader(variant.newReader(builder.source, client));
-    new Thread(readerRunnable).start(); // Not a daemon thread.
   }
 
   /** The protocol as selected using ALPN. */
@@ -501,16 +500,27 @@ private void close(ErrorCode connectionCode, ErrorCode streamCode) throws IOExce
   }
 
   /**
-   * Sends a connection header if the current variant requires it. This should be called after
-   * {@link Builder#build} for all new connections.
+   * Sends any initial frames and starts reading frames from the remote peer. This should be called
+   * after {@link Builder#build} for all new connections.
    */
-  public void sendConnectionPreface() throws IOException {
-    frameWriter.connectionPreface();
-    frameWriter.settings(okHttpSettings);
-    int windowSize = okHttpSettings.getInitialWindowSize(Settings.DEFAULT_INITIAL_WINDOW_SIZE);
-    if (windowSize != Settings.DEFAULT_INITIAL_WINDOW_SIZE) {
-      frameWriter.windowUpdate(0, windowSize - Settings.DEFAULT_INITIAL_WINDOW_SIZE);
+  public void start() throws IOException {
+    start(true);
+  }
+
+  /**
+   * @param sendConnectionPreface true to send connection preface frames. This should always be true
+   *     except for in tests that don't check for a connection preface.
+   */
+  void start(boolean sendConnectionPreface) throws IOException {
+    if (sendConnectionPreface) {
+      frameWriter.connectionPreface();
+      frameWriter.settings(okHttpSettings);
+      int windowSize = okHttpSettings.getInitialWindowSize(Settings.DEFAULT_INITIAL_WINDOW_SIZE);
+      if (windowSize != Settings.DEFAULT_INITIAL_WINDOW_SIZE) {
+        frameWriter.windowUpdate(0, windowSize - Settings.DEFAULT_INITIAL_WINDOW_SIZE);
+      }
     }
+    new Thread(readerRunnable).start(); // Not a daemon thread.
   }
 
   /** Merges {@code settings} into this peer's settings and sends them to the remote peer. */

okhttp/src/main/java/okhttp3/internal/io/RealConnection.java

@@ -159,7 +159,7 @@ private void connectSocket(int connectTimeout, int readTimeout, int writeTimeout
           .protocol(protocol)
           .listener(this)
           .build();
-      framedConnection.sendConnectionPreface();
+      framedConnection.start();
 
       // Only assign the framed connection once the preface has been sent successfully.
       this.allocationLimit = framedConnection.maxConcurrentStreams();