Fix for Bug#88242 (27040063), autoReconnect and socketTimeout JDBC op…

…tion makes wrong order of client packet.
konstantinosss · Nov 15, 2017 · 765f3f6 · 765f3f6
1 parent 650de8b
commit 765f3f6
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 9 deletions.
diff --git a/CHANGES b/CHANGES
@@ -3,6 +3,8 @@
 
 Version 5.1.45
 
+  - Fix for Bug#88242 (27040063), autoReconnect and socketTimeout JDBC option makes wrong order of client packet.
+
   - Fix for Bug#88021 (26939943), High GC pressure when driver configured with serversideprepared statements.
     Thanks to Johnathan Crawford for his contribution.
 

diff --git a/src/com/mysql/jdbc/ConnectionImpl.java b/src/com/mysql/jdbc/ConnectionImpl.java
@@ -2459,7 +2459,7 @@ public ResultSetInternalMethods execSQL(StatementImpl callingStatement, String s
 
             this.lastQueryFinishedTime = 0; // we're busy!
 
-            if ((getHighAvailability()) && (this.autoCommit || getAutoReconnectForPools()) && this.needsPing && !isBatch) {
+            if (getHighAvailability() && (this.autoCommit || getAutoReconnectForPools()) && this.needsPing && !isBatch) {
                 try {
                     pingInternal(false, 0);
 
@@ -2496,19 +2496,23 @@ public ResultSetInternalMethods execSQL(StatementImpl callingStatement, String s
                     sqlE = appendMessageToException(sqlE, messageBuf.toString(), getExceptionInterceptor());
                 }
 
-                if ((getHighAvailability())) {
-                    this.needsPing = true;
-                } else {
-                    String sqlState = sqlE.getSQLState();
-
-                    if ((sqlState != null) && sqlState.equals(SQLError.SQL_STATE_COMMUNICATION_LINK_FAILURE)) {
-                        cleanup(sqlE);
+                if (getHighAvailability()) {
+                    if (SQLError.SQL_STATE_COMMUNICATION_LINK_FAILURE.equals(sqlE.getSQLState())) {
+                        // IO may be dirty or damaged beyond repair, force close it.
+                        this.io.forceClose();
                     }
+                    this.needsPing = true;
+                } else if (SQLError.SQL_STATE_COMMUNICATION_LINK_FAILURE.equals(sqlE.getSQLState())) {
+                    cleanup(sqlE);
                 }
 
                 throw sqlE;
             } catch (Exception ex) {
                 if (getHighAvailability()) {
+                    if (ex instanceof IOException) {
+                        // IO may be dirty or damaged beyond repair, force close it.
+                        this.io.forceClose();
+                    }
                     this.needsPing = true;
                 } else if (ex instanceof IOException) {
                     cleanup(ex);

diff --git a/src/com/mysql/jdbc/MysqlIO.java b/src/com/mysql/jdbc/MysqlIO.java
@@ -480,7 +480,7 @@ protected ResultSetImpl getResultSet(StatementImpl callingStatement, long column
     }
 
     // We do this to break the chain between MysqlIO and Connection, so that we can have PhantomReferences on connections that let the driver clean up the
-    // socket connection without having to use finalize() somewhere (which although more straightforward, is horribly inefficent).
+    // socket connection without having to use finalize() somewhere (which although more straightforward, is horribly inefficient).
     protected NetworkResources getNetworkResources() {
         return new NetworkResources(this.mysqlConnection, this.mysqlInput, this.mysqlOutput);
     }

diff --git a/src/testsuite/regression/ConnectionRegressionTest.java b/src/testsuite/regression/ConnectionRegressionTest.java
@@ -10047,4 +10047,48 @@ public void testBug70785() throws Exception {
             this.stmt.execute("SET @@global.autocommit=" + (originalAutoCommit ? 1 : 0));
         }
     }
+
+    /**
+     * Tests fix for Bug#88242 - autoReconnect and socketTimeout JDBC option makes wrong order of client packet.
+     * 
+     * The wrong behavior may not be observed in all systems or configurations. It seems to be easier to reproduce when SSL is enabled. Without it, the data
+     * packets flow faster and desynchronization occurs rarely, which is the root cause for this problem.
+     */
+    public void testBug88242() throws Exception {
+        Properties props = new Properties();
+        props.setProperty("useSSL", "true");
+        props.setProperty("verifyServerCertificate", "false");
+        props.setProperty("autoReconnect", "true");
+        props.setProperty("socketTimeout", "1500");
+
+        Connection testConn = getConnectionWithProps(props);
+        this.pstmt = testConn.prepareStatement("SELECT ?, SLEEP(?)");
+
+        int key = 0;
+        for (int i = 0; i < 5; i++) {
+            // Execute a query that runs faster than the socket timeout limit.
+            this.pstmt.setInt(1, ++key);
+            this.pstmt.setInt(2, 0);
+            try {
+                this.rs = this.pstmt.executeQuery();
+                assertTrue(this.rs.next());
+                assertEquals(key, this.rs.getInt(1));
+            } catch (SQLException e) {
+                fail("Exception [" + e.getClass().getName() + ": " + e.getMessage() + "] caught when no exception was expected.");
+            }
+
+            // Execute a query that runs slower than the socket timeout limit.
+            this.pstmt.setInt(1, ++key);
+            this.pstmt.setInt(2, 2);
+            final PreparedStatement localPstmt = this.pstmt;
+            assertThrows("Communications link failure.*", SQLException.class, new Callable<Void>() {
+                public Void call() throws Exception {
+                    localPstmt.executeQuery();
+                    return null;
+                }
+            });
+        }
+
+        testConn.close();
+    }
 }