Skip to content

Commit

Permalink
Fix for Bug#84783 (25490163), query timeout is not working(thread hang).
Browse files Browse the repository at this point in the history
  • Loading branch information
@fjssilva
fjssilva committed Apr 4, 2017
1 parent 9e3c246 commit 9ec7f65
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 72 deletions.
2 changes: 2 additions & 0 deletions CHANGES
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# $Id$
mm-dd-yy - Version 5.1.42

- Fix for Bug#84783 (25490163), query timeout is not working(thread hang).

- Fix for Bug#70704 (17653733), Deadlock using UpdatableResultSet.

- Fix for Bug#66430 (16714868), setCatalog on connection leaves ServerPreparedStatement cache for old catalog.
Expand Down
40 changes: 20 additions & 20 deletions src/com/mysql/fabric/jdbc/FabricMySQLConnectionProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,15 +466,15 @@ protected void setCurrentServerGroup(String serverGroupName) throws SQLException
* instead the {@link LoadBalancedConnectionProxy} for either the
* master or slaves.
*/
protected MySQLConnection getActiveMySQLConnection() throws SQLException {
protected MySQLConnection getActiveMySQLConnectionChecked() throws SQLException {
ReplicationConnection c = (ReplicationConnection) getActiveConnection();
MySQLConnection mc = (MySQLConnection) c.getCurrentConnection();
return mc;
}

protected MySQLConnection getActiveMySQLConnectionPassive() {
public MySQLConnection getActiveMySQLConnection() {
try {
return getActiveMySQLConnection();
return getActiveMySQLConnectionChecked();
} catch (SQLException ex) {
throw new IllegalStateException("Unable to determine active connection", ex);
}
Expand Down Expand Up @@ -724,7 +724,7 @@ public MySQLConnection getLoadBalanceSafeProxy() {
}

public MySQLConnection getMultiHostSafeProxy() {
return getActiveMySQLConnectionPassive();
return getActiveMySQLConnection();
}

////////////////////////////////////////////////////////
Expand Down Expand Up @@ -891,30 +891,30 @@ public Statement createStatement(int resultSetType, int resultSetConcurrency, in

public ResultSetInternalMethods execSQL(StatementImpl callingStatement, String sql, int maxRows, Buffer packet, int resultSetType, int resultSetConcurrency,
boolean streamResults, String catalog, Field[] cachedMetadata) throws SQLException {
return getActiveMySQLConnection().execSQL(callingStatement, sql, maxRows, packet, resultSetType, resultSetConcurrency, streamResults, catalog,
return getActiveMySQLConnectionChecked().execSQL(callingStatement, sql, maxRows, packet, resultSetType, resultSetConcurrency, streamResults, catalog,
cachedMetadata);
}

public ResultSetInternalMethods execSQL(StatementImpl callingStatement, String sql, int maxRows, Buffer packet, int resultSetType, int resultSetConcurrency,
boolean streamResults, String catalog, Field[] cachedMetadata, boolean isBatch) throws SQLException {
return getActiveMySQLConnection().execSQL(callingStatement, sql, maxRows, packet, resultSetType, resultSetConcurrency, streamResults, catalog,
return getActiveMySQLConnectionChecked().execSQL(callingStatement, sql, maxRows, packet, resultSetType, resultSetConcurrency, streamResults, catalog,
cachedMetadata, isBatch);
}

public String extractSqlFromPacket(String possibleSqlQuery, Buffer queryPacket, int endOfQueryPacketPosition) throws SQLException {
return getActiveMySQLConnection().extractSqlFromPacket(possibleSqlQuery, queryPacket, endOfQueryPacketPosition);
return getActiveMySQLConnectionChecked().extractSqlFromPacket(possibleSqlQuery, queryPacket, endOfQueryPacketPosition);
}

public StringBuilder generateConnectionCommentBlock(StringBuilder buf) {
return getActiveMySQLConnectionPassive().generateConnectionCommentBlock(buf);
return getActiveMySQLConnection().generateConnectionCommentBlock(buf);
}

public MysqlIO getIO() throws SQLException {
return getActiveMySQLConnection().getIO();
return getActiveMySQLConnectionChecked().getIO();
}

public Calendar getCalendarInstanceForSessionOrNew() {
return getActiveMySQLConnectionPassive().getCalendarInstanceForSessionOrNew();
return getActiveMySQLConnection().getCalendarInstanceForSessionOrNew();
}

/**
Expand All @@ -926,11 +926,11 @@ public String getServerCharacterEncoding() {
}

public String getServerCharset() {
return getActiveMySQLConnectionPassive().getServerCharset();
return getActiveMySQLConnection().getServerCharset();
}

public TimeZone getServerTimezoneTZ() {
return getActiveMySQLConnectionPassive().getServerTimezoneTZ();
return getActiveMySQLConnection().getServerTimezoneTZ();
}

/**
Expand Down Expand Up @@ -960,11 +960,11 @@ public DatabaseMetaData getMetaData() throws SQLException {
}

public String getCharacterSetMetadata() {
return getActiveMySQLConnectionPassive().getCharacterSetMetadata();
return getActiveMySQLConnection().getCharacterSetMetadata();
}

public java.sql.Statement getMetadataSafeStatement() throws SQLException {
return getActiveMySQLConnection().getMetadataSafeStatement();
return getActiveMySQLConnectionChecked().getMetadataSafeStatement();
}

/**
Expand Down Expand Up @@ -2856,7 +2856,7 @@ public String getHost() {
}

public String getHostPortPair() {
return getActiveMySQLConnectionPassive().getHostPortPair();
return getActiveMySQLConnection().getHostPortPair();
}

public long getId() {
Expand Down Expand Up @@ -2955,7 +2955,7 @@ public boolean isServerTzUTC() {
}

public boolean lowerCaseTableNames() {
return getActiveMySQLConnectionPassive().lowerCaseTableNames();
return getActiveMySQLConnection().lowerCaseTableNames();
}

/**
Expand Down Expand Up @@ -2984,7 +2984,7 @@ public void reportNumberOfTablesAccessed(int numTablesAccessed) {
}

public boolean serverSupportsConvertFn() throws SQLException {
return getActiveMySQLConnection().serverSupportsConvertFn();
return getActiveMySQLConnectionChecked().serverSupportsConvertFn();
}

public void setReadInfoMsgEnabled(boolean flag) {
Expand All @@ -2994,7 +2994,7 @@ public void setReadOnlyInternal(boolean readOnlyFlag) throws SQLException {
}

public boolean storesLowerCaseTableName() {
return getActiveMySQLConnectionPassive().storesLowerCaseTableName();
return getActiveMySQLConnection().storesLowerCaseTableName();
}

public void throwConnectionClosedException() throws SQLException {
Expand Down Expand Up @@ -3049,11 +3049,11 @@ public Map<String, Class<?>> getTypeMap() {
}

public SQLWarning getWarnings() throws SQLException {
return getActiveMySQLConnection().getWarnings();
return getActiveMySQLConnectionChecked().getWarnings();
}

public String nativeSQL(String sql) throws SQLException {
return getActiveMySQLConnection().nativeSQL(sql);
return getActiveMySQLConnectionChecked().nativeSQL(sql);
}

public ProfilerEventHandler getProfilerEventHandlerInstance() {
Expand Down
22 changes: 5 additions & 17 deletions src/com/mysql/jdbc/ConnectionImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ public MySQLConnection getMultiHostSafeProxy() {
return this.getProxy();
}

public MySQLConnection getActiveMySQLConnection() {
return this;
}

public Object getConnectionMutex() {
return (this.realProxy != null) ? this.realProxy : getProxy();
}
Expand Down Expand Up @@ -363,23 +367,8 @@ protected static SQLException appendMessageToException(SQLException sqlEx, Strin
public Timer getCancelTimer() {
synchronized (getConnectionMutex()) {
if (this.cancelTimer == null) {
boolean createdNamedTimer = false;

// Use reflection magic to try this on JDK's 1.5 and newer, fallback to non-named timer on older VMs.
try {
Constructor<Timer> ctr = Timer.class.getConstructor(new Class[] { String.class, Boolean.TYPE });

this.cancelTimer = ctr.newInstance(new Object[] { "MySQL Statement Cancellation Timer", Boolean.TRUE });
createdNamedTimer = true;
} catch (Throwable t) {
createdNamedTimer = false;
}

if (!createdNamedTimer) {
this.cancelTimer = new Timer(true);
}
this.cancelTimer = new Timer("MySQL Statement Cancellation Timer", true);
}

return this.cancelTimer;
}
}
Expand Down Expand Up @@ -4654,7 +4643,6 @@ private void rollbackNoChecks() throws SQLException {
* @see java.sql.Connection#prepareStatement(String)
*/
public java.sql.PreparedStatement serverPrepareStatement(String sql) throws SQLException {

String nativeSql = getProcessEscapeCodesForPrepStmts() ? nativeSQL(sql) : sql;

return ServerPreparedStatement.getInstance(getMultiHostSafeProxy(), nativeSql, this.getCatalog(), DEFAULT_RESULT_SET_TYPE,
Expand Down
4 changes: 2 additions & 2 deletions src/com/mysql/jdbc/MultiHostMySQLConnection.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
The MySQL Connector/J is licensed under the terms of the GPLv2
<http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>, like most MySQL Connectors.
Expand Down Expand Up @@ -70,7 +70,7 @@ protected MultiHostConnectionProxy getThisAsProxy() {
return this.thisAsProxy;
}

protected MySQLConnection getActiveMySQLConnection() {
public MySQLConnection getActiveMySQLConnection() {
synchronized (this.thisAsProxy) {
return this.thisAsProxy.currentConnection;
}
Expand Down
4 changes: 3 additions & 1 deletion src/com/mysql/jdbc/MySQLConnection.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2010, 2016, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2010, 2017, Oracle and/or its affiliates. All rights reserved.
The MySQL Connector/J is licensed under the terms of the GPLv2
<http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>, like most MySQL Connectors.
Expand Down Expand Up @@ -207,6 +207,8 @@ ResultSetInternalMethods execSQL(StatementImpl callingStatement, String sql, int

MySQLConnection getMultiHostSafeProxy();

MySQLConnection getActiveMySQLConnection();

ProfilerEventHandler getProfilerEventHandlerInstance();

void setProfilerEventHandlerInstance(ProfilerEventHandler h);
Expand Down
4 changes: 2 additions & 2 deletions src/com/mysql/jdbc/ReplicationMySQLConnection.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
The MySQL Connector/J is licensed under the terms of the GPLv2
<http://www.gnu.org/licenses/old-licenses/gpl-2.0.html>, like most MySQL Connectors.
Expand Down Expand Up @@ -38,7 +38,7 @@ protected ReplicationConnectionProxy getThisAsProxy() {
}

@Override
protected MySQLConnection getActiveMySQLConnection() {
public MySQLConnection getActiveMySQLConnection() {
return (MySQLConnection) getCurrentConnection();
}

Expand Down
68 changes: 40 additions & 28 deletions src/com/mysql/jdbc/StatementImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
package com.mysql.jdbc;

import java.io.InputStream;
import java.lang.ref.Reference;
import java.lang.ref.WeakReference;
import java.math.BigInteger;
import java.sql.BatchUpdateException;
import java.sql.DriverManager;
Expand Down Expand Up @@ -67,15 +69,13 @@ public class StatementImpl implements Statement {
* and simple way to implement a feature that isn't used all that often.
*/
class CancelTask extends TimerTask {

long connectionId = 0;
SQLException caughtWhileCancelling = null;
StatementImpl toCancel;
Properties origConnProps = null;
String origConnURL = "";
long origConnId = 0;

CancelTask(StatementImpl cancellee) throws SQLException {
this.connectionId = cancellee.connectionId;
this.toCancel = cancellee;
this.origConnProps = new Properties();

Expand All @@ -89,6 +89,7 @@ class CancelTask extends TimerTask {
}

this.origConnURL = StatementImpl.this.connection.getURL();
this.origConnId = StatementImpl.this.connection.getId();
}

@Override
Expand All @@ -103,38 +104,40 @@ public void run() {
java.sql.Statement cancelStmt = null;

try {
if (StatementImpl.this.connection.getQueryTimeoutKillsConnection()) {
CancelTask.this.toCancel.wasCancelled = true;
CancelTask.this.toCancel.wasCancelledByTimeout = true;
StatementImpl.this.connection.realClose(false, false, true,
new MySQLStatementCancelledException(Messages.getString("Statement.ConnectionKilledDueToTimeout")));
} else {
synchronized (StatementImpl.this.cancelTimeoutMutex) {
if (CancelTask.this.origConnURL.equals(StatementImpl.this.connection.getURL())) {
//All's fine
cancelConn = StatementImpl.this.connection.duplicate();
cancelStmt = cancelConn.createStatement();
cancelStmt.execute("KILL QUERY " + CancelTask.this.connectionId);
} else {
try {
cancelConn = (Connection) DriverManager.getConnection(CancelTask.this.origConnURL, CancelTask.this.origConnProps);
MySQLConnection physicalConn = StatementImpl.this.physicalConnection.get();
if (physicalConn != null) {
if (physicalConn.getQueryTimeoutKillsConnection()) {
CancelTask.this.toCancel.wasCancelled = true;
CancelTask.this.toCancel.wasCancelledByTimeout = true;
physicalConn.realClose(false, false, true,
new MySQLStatementCancelledException(Messages.getString("Statement.ConnectionKilledDueToTimeout")));
} else {
synchronized (StatementImpl.this.cancelTimeoutMutex) {
if (CancelTask.this.origConnURL.equals(physicalConn.getURL())) {
// All's fine
cancelConn = physicalConn.duplicate();
cancelStmt = cancelConn.createStatement();
cancelStmt.execute("KILL QUERY " + CancelTask.this.connectionId);
} catch (NullPointerException npe) {
//Log this? "Failed to connect to " + origConnURL + " and KILL query"
cancelStmt.execute("KILL QUERY " + physicalConn.getId());
} else {
try {
cancelConn = (Connection) DriverManager.getConnection(CancelTask.this.origConnURL, CancelTask.this.origConnProps);
cancelStmt = cancelConn.createStatement();
cancelStmt.execute("KILL QUERY " + CancelTask.this.origConnId);
} catch (NullPointerException npe) {
// Log this? "Failed to connect to " + origConnURL + " and KILL query"
}
}
CancelTask.this.toCancel.wasCancelled = true;
CancelTask.this.toCancel.wasCancelledByTimeout = true;
}
CancelTask.this.toCancel.wasCancelled = true;
CancelTask.this.toCancel.wasCancelledByTimeout = true;
}
}
} catch (SQLException sqlEx) {
CancelTask.this.caughtWhileCancelling = sqlEx;
} catch (NullPointerException npe) {
// Case when connection closed while starting to cancel
// We can't easily synchronize this, because then one thread can't cancel() a running query

// ignore, we shouldn't re-throw this, because the connection's already closed, so the statement has been timed out.
// Case when connection closed while starting to cancel.
// We can't easily synchronize this, because then one thread can't cancel() a running query.
// Ignore, we shouldn't re-throw this, because the connection's already closed, so the statement has been timed out.
} finally {
if (cancelStmt != null) {
try {
Expand Down Expand Up @@ -194,6 +197,9 @@ public void run() {
/** The connection that created us */
protected volatile MySQLConnection connection = null;

/** The physical connection used to effectively execute the statement */
protected Reference<MySQLConnection> physicalConnection = null;

protected long connectionId = 0;

/** The catalog in use */
Expand Down Expand Up @@ -300,7 +306,7 @@ public void run() {
* Constructor for a Statement.
*
* @param c
* the Connection instantation that creates us
* the Connection instance that creates us
* @param catalog
* the database name in use when we were created
*
Expand Down Expand Up @@ -905,6 +911,12 @@ private boolean executeInternal(String sql, boolean returnGeneratedKeys) throws
protected void statementBegins() {
this.clearWarningsCalled = false;
this.statementExecuting.set(true);

MySQLConnection physicalConn = this.connection.getMultiHostSafeProxy().getActiveMySQLConnection();
while (!(physicalConn instanceof ConnectionImpl)) {
physicalConn = physicalConn.getMultiHostSafeProxy().getActiveMySQLConnection();
}
this.physicalConnection = new WeakReference<MySQLConnection>(physicalConn);
}

protected void resetCancelledState() throws SQLException {
Expand Down
Loading

0 comments on commit 9ec7f65

Please sign in to comment.