Fixing okhttp hang when tls nego fails.

Negotiation failure results in a RuntimeException, which is not properly  handled by the okhttp code, resulting in the client hanging.

Refactored the code to shutdown the transport when TLS negotiation fails.

okhttp/src/main/java/com/squareup/okhttp/OkHttpTlsUpgrader.java

@@ -56,6 +56,9 @@ public final class OkHttpTlsUpgrader {
 
   /**
    * Upgrades given Socket to be a SSLSocket.
+   *
+   * @throws IOException if an IO error was encountered during the upgrade handshake.
+   * @throws RuntimeException if the upgrade negotiation failed.
    */
   public static SSLSocket upgrade(SSLSocketFactory sslSocketFactory,
       Socket socket, String host, int port, ConnectionSpec spec) throws IOException {

okhttp/src/main/java/com/squareup/okhttp/internal/OkHttpProtocolNegotiator.java

@@ -75,6 +75,9 @@ public class OkHttpProtocolNegotiator {
 
   /**
    * Start and wait until the negotiation is done, returns the negotiated protocol.
+   *
+   * @throws IOException if an IO error was encountered during the handshake.
+   * @throws RuntimeException if the negotiation completed, but no protocol was selected.
    */
   public String negotiate(
       SSLSocket sslSocket, String hostname, List<Protocol> protocols) throws IOException {

okhttp/src/main/java/io/grpc/okhttp/AsyncFrameWriter.java

@@ -237,9 +237,12 @@ class AsyncFrameWriter implements FrameWriter {
           throw new IOException("Unable to perform write due to unavailable frameWriter.");
         }
         doRun();
-      } catch (IOException ex) {
-        transport.onIoException(ex);
-        throw new RuntimeException(ex);
+      } catch (RuntimeException e) {
+        transport.onException(e);
+        throw e;
+      } catch (Exception e) {
+        transport.onException(e);
+        throw new RuntimeException(e);
       }
     }
 

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java

@@ -357,8 +357,12 @@ class OkHttpClientTransport implements ClientTransport {
           sock.setTcpNoDelay(true);
           source = Okio.buffer(Okio.source(sock));
           sink = Okio.buffer(Okio.sink(sock));
-        } catch (IOException e) {
-          onIoException(e);
+        } catch (RuntimeException e) {
+          onException(e);
+          throw e;
+        } catch (Exception e) {
+          onException(e);
+
           // (and probably do all of this work asynchronously instead of in calling thread)
           throw new RuntimeException(e);
         }
@@ -388,8 +392,11 @@ class OkHttpClientTransport implements ClientTransport {
           rawFrameWriter.connectionPreface();
           Settings settings = new Settings();
           rawFrameWriter.settings(settings);
-        } catch (IOException e) {
-          onIoException(e);
+        } catch (RuntimeException e) {
+          onException(e);
+          throw e;
+        } catch (Exception e) {
+          onException(e);
           throw new RuntimeException(e);
         }
 
@@ -439,7 +446,7 @@ class OkHttpClientTransport implements ClientTransport {
   /**
    * Finish all active streams due to an IOException, then close the transport.
    */
-  void onIoException(IOException failureCause) {
+  void onException(Throwable failureCause) {
     log.log(Level.SEVERE, "Transport failed", failureCause);
     onGoAway(0, Status.UNAVAILABLE.withCause(failureCause));
   }
@@ -587,11 +594,10 @@ class OkHttpClientTransport implements ClientTransport {
         // Read until the underlying socket closes.
         while (frameReader.nextFrame(this)) {
         }
-      } catch (IOException ioe) {
-        // We send GoAway here because OkHttp wraps many protocol errors as IOException.
+      } catch (Exception t) {
         // TODO(madongfly): Send the exception message to the server.
         frameWriter.goAway(0, ErrorCode.PROTOCOL_ERROR, new byte[0]);
-        onIoException(ioe);
+        onException(t);
       } finally {
         try {
           frameReader.close();

okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java

@@ -1087,7 +1087,7 @@ public class OkHttpClientTransportTest {
     clientTransport.ping(callback, MoreExecutors.directExecutor());
     assertEquals(0, callback.invocationCount);
 
-    clientTransport.onIoException(new IOException());
+    clientTransport.onException(new IOException());
     // ping failed on error
     assertEquals(1, callback.invocationCount);
     assertTrue(callback.failureCause instanceof StatusException);