From 34520911e3b37d3f4a8e6ff1ba1c368b70ac4a3b Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Wed, 22 Nov 2017 14:46:28 -0800 Subject: [PATCH 1/2] new property for cancelTimeout to cancel the query timeout --- .../microsoft/sqlserver/jdbc/IOBuffer.java | 43 ++++++++++++++++--- .../sqlserver/jdbc/SQLServerConnection.java | 34 +++++++++++++++ .../sqlserver/jdbc/SQLServerDataSource.java | 20 +++++++++ .../sqlserver/jdbc/SQLServerDriver.java | 2 + .../sqlserver/jdbc/SQLServerResource.java | 2 + 5 files changed, 94 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index d87a9cb14..405b6349c 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -6357,7 +6357,8 @@ final class TDSReaderMark { final class TDSReader { private final static Logger logger = Logger.getLogger("com.microsoft.sqlserver.jdbc.internals.TDS.Reader"); final private String traceID; - + private TimeoutTimer tcpKeepAliveTimeoutTimer; + final public String toString() { return traceID; } @@ -6398,6 +6399,8 @@ private static int nextReaderID() { this.tdsChannel = tdsChannel; this.con = con; this.command = command; // may be null + //if cancelTimeout is set, we should wait for the total amount of queryTimeout+cancelTimeout to terminate the connection. + this.tcpKeepAliveTimeoutTimer = (con.getCancelTimeoutSeconds() > 0 && con.getQueryTimeoutSeconds()> 0 ) ? (new TimeoutTimer(con.getCancelTimeoutSeconds() + con.getQueryTimeoutSeconds(), null, con)) : null; // if the logging level is not detailed than fine or more we will not have proper readerids. if (logger.isLoggable(Level.FINE)) traceID = "TDSReader@" + nextReaderID() + " (" + con.toString() + ")"; @@ -6496,7 +6499,12 @@ synchronized final boolean readPacket() throws SQLServerException { + tdsChannel.numMsgsSent; TDSPacket newPacket = new TDSPacket(con.getTDSPacketSize()); - + if (null != tcpKeepAliveTimeoutTimer) { + if (logger.isLoggable(Level.FINEST)) { + logger.finest(this.toString() + ":starting timer..."); + } + tcpKeepAliveTimeoutTimer.start(); + } // First, read the packet header. for (int headerBytesRead = 0; headerBytesRead < TDS.PACKET_HEADER_SIZE;) { int bytesRead = tdsChannel.read(newPacket.header, headerBytesRead, TDS.PACKET_HEADER_SIZE - headerBytesRead); @@ -6510,7 +6518,14 @@ synchronized final boolean readPacket() throws SQLServerException { headerBytesRead += bytesRead; } - + + // if execution was subject to timeout then stop timing + if (null != tcpKeepAliveTimeoutTimer) { + if (logger.isLoggable(Level.FINEST)) { + logger.finest(this.toString() + ":stopping timer..."); + } + tcpKeepAliveTimeoutTimer.stop(); + } // Header size is a 2 byte unsigned short integer in big-endian order. int packetLength = Util.readUnsignedShortBigEndian(newPacket.header, TDS.PACKET_HEADER_MESSAGE_LENGTH); @@ -7118,7 +7133,8 @@ final class TimeoutTimer implements Runnable { private final int timeoutSeconds; private final TDSCommand command; private volatile Future task; - + private final SQLServerConnection con; + private static final ExecutorService executor = Executors.newCachedThreadPool(new ThreadFactory() { private final AtomicReference tgr = new AtomicReference<>(); private final AtomicInteger threadNumber = new AtomicInteger(0); @@ -7143,12 +7159,14 @@ public Thread newThread(Runnable r) private volatile boolean canceled = false; TimeoutTimer(int timeoutSeconds, - TDSCommand command) { + TDSCommand command, + SQLServerConnection con) { assert timeoutSeconds > 0; assert null != command; this.timeoutSeconds = timeoutSeconds; this.command = command; + this.con = con; } final void start() { @@ -7182,12 +7200,23 @@ public void run() { // If the timer wasn't canceled before it ran out of // time then interrupt the registered command. try { - command.interrupt(SQLServerException.getErrString("R_queryTimedOut")); + // if connection is silently dropped, the query timeout hangs too and does not throw the query timeout exception. + // The application hangs until the connection timeout is thrown. In this case, manually terminate the connection. + if (null == command) { + assert null != con; + con.terminate(SQLServerException.DRIVER_ERROR_IO_FAILED, SQLServerException.getErrString("R_connectionIsClosed")); + } + else { + // If the timer wasn't canceled before it ran out of + // time then interrupt the registered command. + command.interrupt(SQLServerException.getErrString("R_queryTimedOut")); + } } catch (SQLServerException e) { // Unfortunately, there's nothing we can do if we // fail to time out the request. There is no way // to report back what happened. + assert null != command; command.log(Level.FINE, "Command could not be timed out. Reason: " + e.getMessage()); } } @@ -7331,7 +7360,7 @@ final boolean readingResponse() { TDSCommand(String logContext, int timeoutSeconds) { this.logContext = logContext; - this.timeoutTimer = (timeoutSeconds > 0) ? (new TimeoutTimer(timeoutSeconds, this)) : null; + this.timeoutTimer = (timeoutSeconds > 0) ? (new TimeoutTimer(timeoutSeconds, this, null)) : null; } /** diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 241c93688..38d7afd8a 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -466,6 +466,18 @@ final String getResponseBuffering() { final int getQueryTimeoutSeconds() { return queryTimeoutSeconds; } + /** + * timeout value for canceling the query timeout + */ + private int cancelTimeoutSeconds; + + /** + * Retrieves the cancelTimeout in seconds + * @return + */ + final int getCancelTimeoutSeconds() { + return cancelTimeoutSeconds; + } private int socketTimeoutMilliseconds; @@ -1698,6 +1710,28 @@ else if (0 == requestedPacketSize) } } + sPropKey = SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(); + int defaultCancelTimeout = SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue(); + // use cancelTimeout only if queryTimeout is set. + if (activeConnectionProperties.getProperty(sPropKey) != null && activeConnectionProperties.getProperty(sPropKey).length() > 0 && queryTimeoutSeconds > defaultQueryTimeout) { + try { + int n = new Integer(activeConnectionProperties.getProperty(sPropKey)); + if (n >= defaultCancelTimeout) { + cancelTimeoutSeconds = n; + } + else { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelTimeout")); + Object[] msgArgs = {activeConnectionProperties.getProperty(sPropKey)}; + SQLServerException.makeFromDriverError(this, this, form.format(msgArgs), null, false); + } + } + catch (NumberFormatException e) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelTimeout")); + Object[] msgArgs = {activeConnectionProperties.getProperty(sPropKey)}; + SQLServerException.makeFromDriverError(this, this, form.format(msgArgs), null, false); + } + } + sPropKey = SQLServerDriverIntProperty.SERVER_PREPARED_STATEMENT_DISCARD_THRESHOLD.toString(); if (activeConnectionProperties.getProperty(sPropKey) != null && activeConnectionProperties.getProperty(sPropKey).length() > 0) { try { diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java index 49a6b25ba..3328e95c6 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java @@ -690,6 +690,26 @@ public int getQueryTimeout() { return getIntProperty(connectionProps, SQLServerDriverIntProperty.QUERY_TIMEOUT.toString(), SQLServerDriverIntProperty.QUERY_TIMEOUT.getDefaultValue()); } + + /** + * Setting the cancel timeout + * + * @param cancelTimeout + * The number of seconds to wait before we wait for the query timeout to happen. + */ + public void setCancelTimeout(int cancelTimeout) { + setIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), cancelTimeout); + } + + /** + * Getting the cancel timeout + * + * @return the number of seconds to wait before we wait for the query timeout to happen. + */ + public int getCancelTimeout() { + return getIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), + SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue()); + } /** * If this configuration is false the first execution of a prepared statement will call sp_executesql and not prepare diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java index 1dfeae276..325130294 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java @@ -318,6 +318,7 @@ enum SQLServerDriverIntProperty { SOCKET_TIMEOUT ("socketTimeout", 0), SERVER_PREPARED_STATEMENT_DISCARD_THRESHOLD("serverPreparedStatementDiscardThreshold", SQLServerConnection.DEFAULT_SERVER_PREPARED_STATEMENT_DISCARD_THRESHOLD), STATEMENT_POOLING_CACHE_SIZE ("statementPoolingCacheSize", SQLServerConnection.DEFAULT_STATEMENT_POOLING_CACHE_SIZE), + CANCEL_TIMEOUT ("cancelTimeout", -1), ; private final String name; @@ -428,6 +429,7 @@ public final class SQLServerDriver implements java.sql.Driver { new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.toString(), Integer.toString(SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.getDefaultValue()), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.JAAS_CONFIG_NAME.toString(), SQLServerDriverStringProperty.JAAS_CONFIG_NAME.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.SSL_PROTOCOL.toString(), SQLServerDriverStringProperty.SSL_PROTOCOL.getDefaultValue(), false, new String[] {SSLProtocol.TLS.toString(), SSLProtocol.TLS_V10.toString(), SSLProtocol.TLS_V11.toString(), SSLProtocol.TLS_V12.toString()}), + new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), Integer.toString(SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue()), false, null), }; // Properties that can only be set by using Properties. diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java index 63e701505..16371db0f 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java @@ -392,5 +392,7 @@ protected Object[][] getContents() { {"R_invalidDataTypeSupportForSQLVariant", "Unexpected TDS type ' '{0}' ' in SQL_VARIANT."}, {"R_sslProtocolPropertyDescription", "SSL protocol label from TLS, TLSv1, TLSv1.1 & TLSv1.2. The default is TLS."}, {"R_invalidSSLProtocol", "SSL Protocol {0} label is not valid. Only TLS, TLSv1, TLSv1.1 & TLSv1.2 are supported."}, + {"R_cancelTimeoutPropertyDescription", "The number of seconds to wait to cancel sending a query timeout."}, + {"R_invalidCancelTimeout", "The cancel timeout value {0} is not valid."}, }; } \ No newline at end of file From 0363257ff9f256f8591fd98f6affa5f1916eba9d Mon Sep 17 00:00:00 2001 From: Afsaneh Rafighi Date: Tue, 5 Dec 2017 13:02:45 -0800 Subject: [PATCH 2/2] update the connection property name to cancelQueryTimeout --- .../com/microsoft/sqlserver/jdbc/IOBuffer.java | 2 +- .../sqlserver/jdbc/SQLServerConnection.java | 16 ++++++++-------- .../sqlserver/jdbc/SQLServerDataSource.java | 12 ++++++------ .../sqlserver/jdbc/SQLServerDriver.java | 4 ++-- .../sqlserver/jdbc/SQLServerResource.java | 4 ++-- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index 405b6349c..a2ce8064d 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -6400,7 +6400,7 @@ private static int nextReaderID() { this.con = con; this.command = command; // may be null //if cancelTimeout is set, we should wait for the total amount of queryTimeout+cancelTimeout to terminate the connection. - this.tcpKeepAliveTimeoutTimer = (con.getCancelTimeoutSeconds() > 0 && con.getQueryTimeoutSeconds()> 0 ) ? (new TimeoutTimer(con.getCancelTimeoutSeconds() + con.getQueryTimeoutSeconds(), null, con)) : null; + this.tcpKeepAliveTimeoutTimer = (con.getCancelQueryTimeoutSeconds() > 0 && con.getQueryTimeoutSeconds()> 0 ) ? (new TimeoutTimer(con.getCancelQueryTimeoutSeconds() + con.getQueryTimeoutSeconds(), null, con)) : null; // if the logging level is not detailed than fine or more we will not have proper readerids. if (logger.isLoggable(Level.FINE)) traceID = "TDSReader@" + nextReaderID() + " (" + con.toString() + ")"; diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 38d7afd8a..bbdf88001 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -469,14 +469,14 @@ final int getQueryTimeoutSeconds() { /** * timeout value for canceling the query timeout */ - private int cancelTimeoutSeconds; + private int cancelQueryTimeoutSeconds; /** * Retrieves the cancelTimeout in seconds * @return */ - final int getCancelTimeoutSeconds() { - return cancelTimeoutSeconds; + final int getCancelQueryTimeoutSeconds() { + return cancelQueryTimeoutSeconds; } private int socketTimeoutMilliseconds; @@ -1710,23 +1710,23 @@ else if (0 == requestedPacketSize) } } - sPropKey = SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(); - int defaultCancelTimeout = SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue(); + sPropKey = SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.toString(); + int defaultCancelTimeout = SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.getDefaultValue(); // use cancelTimeout only if queryTimeout is set. if (activeConnectionProperties.getProperty(sPropKey) != null && activeConnectionProperties.getProperty(sPropKey).length() > 0 && queryTimeoutSeconds > defaultQueryTimeout) { try { int n = new Integer(activeConnectionProperties.getProperty(sPropKey)); if (n >= defaultCancelTimeout) { - cancelTimeoutSeconds = n; + cancelQueryTimeoutSeconds = n; } else { - MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelTimeout")); + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelQueryTimeout")); Object[] msgArgs = {activeConnectionProperties.getProperty(sPropKey)}; SQLServerException.makeFromDriverError(this, this, form.format(msgArgs), null, false); } } catch (NumberFormatException e) { - MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelTimeout")); + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidCancelQueryTimeout")); Object[] msgArgs = {activeConnectionProperties.getProperty(sPropKey)}; SQLServerException.makeFromDriverError(this, this, form.format(msgArgs), null, false); } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java index 3328e95c6..8e114af48 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java @@ -694,11 +694,11 @@ public int getQueryTimeout() { /** * Setting the cancel timeout * - * @param cancelTimeout + * @param cancelQueryTimeout * The number of seconds to wait before we wait for the query timeout to happen. */ - public void setCancelTimeout(int cancelTimeout) { - setIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), cancelTimeout); + public void setCancelQueryTimeout(int cancelQueryTimeout) { + setIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.toString(), cancelQueryTimeout); } /** @@ -706,9 +706,9 @@ public void setCancelTimeout(int cancelTimeout) { * * @return the number of seconds to wait before we wait for the query timeout to happen. */ - public int getCancelTimeout() { - return getIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), - SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue()); + public int getCancelQueryTimeout() { + return getIntProperty(connectionProps, SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.toString(), + SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.getDefaultValue()); } /** diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java index 325130294..e4f6f238e 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java @@ -318,7 +318,7 @@ enum SQLServerDriverIntProperty { SOCKET_TIMEOUT ("socketTimeout", 0), SERVER_PREPARED_STATEMENT_DISCARD_THRESHOLD("serverPreparedStatementDiscardThreshold", SQLServerConnection.DEFAULT_SERVER_PREPARED_STATEMENT_DISCARD_THRESHOLD), STATEMENT_POOLING_CACHE_SIZE ("statementPoolingCacheSize", SQLServerConnection.DEFAULT_STATEMENT_POOLING_CACHE_SIZE), - CANCEL_TIMEOUT ("cancelTimeout", -1), + CANCEL_QUERY_TIMEOUT ("cancelQueryTimeout", -1), ; private final String name; @@ -429,7 +429,7 @@ public final class SQLServerDriver implements java.sql.Driver { new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.toString(), Integer.toString(SQLServerDriverIntProperty.STATEMENT_POOLING_CACHE_SIZE.getDefaultValue()), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.JAAS_CONFIG_NAME.toString(), SQLServerDriverStringProperty.JAAS_CONFIG_NAME.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.SSL_PROTOCOL.toString(), SQLServerDriverStringProperty.SSL_PROTOCOL.getDefaultValue(), false, new String[] {SSLProtocol.TLS.toString(), SSLProtocol.TLS_V10.toString(), SSLProtocol.TLS_V11.toString(), SSLProtocol.TLS_V12.toString()}), - new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.CANCEL_TIMEOUT.toString(), Integer.toString(SQLServerDriverIntProperty.CANCEL_TIMEOUT.getDefaultValue()), false, null), + new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.toString(), Integer.toString(SQLServerDriverIntProperty.CANCEL_QUERY_TIMEOUT.getDefaultValue()), false, null), }; // Properties that can only be set by using Properties. diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java index 16371db0f..f217bc992 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java @@ -392,7 +392,7 @@ protected Object[][] getContents() { {"R_invalidDataTypeSupportForSQLVariant", "Unexpected TDS type ' '{0}' ' in SQL_VARIANT."}, {"R_sslProtocolPropertyDescription", "SSL protocol label from TLS, TLSv1, TLSv1.1 & TLSv1.2. The default is TLS."}, {"R_invalidSSLProtocol", "SSL Protocol {0} label is not valid. Only TLS, TLSv1, TLSv1.1 & TLSv1.2 are supported."}, - {"R_cancelTimeoutPropertyDescription", "The number of seconds to wait to cancel sending a query timeout."}, - {"R_invalidCancelTimeout", "The cancel timeout value {0} is not valid."}, + {"R_cancelQueryTimeoutPropertyDescription", "The number of seconds to wait to cancel sending a query timeout."}, + {"R_invalidCancelQueryTimeout", "The cancel timeout value {0} is not valid."}, }; } \ No newline at end of file