Use CommandTimeoutExpired consistently. Fixes #939

bgrainger · bgrainger · commit 2e3fd52eab64 · 2021-02-11T18:24:23.000-08:00
diff --git a/src/MySqlConnector/Core/ICancellableCommand.cs b/src/MySqlConnector/Core/ICancellableCommand.cs
@@ -15,6 +15,7 @@ internal interface ICancellableCommand
 		MySqlConnection? Connection { get; }
 		IDisposable? RegisterCancel(CancellationToken cancellationToken);
 		void SetTimeout(int milliseconds);
+		bool IsTimedOut { get; }
 	}
 
 	internal static class ICancellableCommandExtensions
diff --git a/src/MySqlConnector/Core/ResultSet.cs b/src/MySqlConnector/Core/ResultSet.cs
@@ -248,6 +248,8 @@ public async Task<bool> ReadAsync(IOBehavior ioBehavior, CancellationToken cance
 					resultSet.BufferState = resultSet.State = ResultSetState.NoMoreData;
 					if (ex.ErrorCode == MySqlErrorCode.QueryInterrupted && token.IsCancellationRequested)
 						throw new OperationCanceledException(ex.Message, ex, token);
+					if (ex.ErrorCode == MySqlErrorCode.QueryInterrupted && resultSet.Command.CancellableCommand.IsTimedOut)
+						throw MySqlException.CreateForTimeout(ex);
 					throw;
 				}
 				return ScanRowAsyncRemainder(resultSet, payloadData, row);
diff --git a/src/MySqlConnector/MySqlBatch.cs b/src/MySqlConnector/MySqlBatch.cs
@@ -173,16 +173,26 @@ public void Dispose()
 
 		void ICancellableCommand.SetTimeout(int milliseconds)
 		{
+			Volatile.Write(ref m_commandTimedOut, false);
+
 			if (m_cancelTimerId != 0)
 				TimerQueue.Instance.Remove(m_cancelTimerId);
 
 			if (milliseconds != Constants.InfiniteTimeout)
 			{
-				m_cancelAction ??= Cancel;
-				m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelAction);
+				m_cancelForCommandTimeoutAction ??= CancelCommandForTimeout;
+				m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelForCommandTimeoutAction);
 			}
 		}
 
+		bool ICancellableCommand.IsTimedOut => Volatile.Read(ref m_commandTimedOut);
+
+		private void CancelCommandForTimeout()
+		{
+			Volatile.Write(ref m_commandTimedOut, true);
+			Cancel();
+		}
+
 		private async Task<int> ExecuteNonQueryAsync(IOBehavior ioBehavior, CancellationToken cancellationToken)
 		{
 			((ICancellableCommand) this).ResetCommandTimeout();
@@ -309,6 +319,8 @@ private bool IsPrepared
 		readonly int m_commandId;
 		bool m_isDisposed;
 		Action? m_cancelAction;
+		Action? m_cancelForCommandTimeoutAction;
 		uint m_cancelTimerId;
+		bool m_commandTimedOut;
 	}
 }
diff --git a/src/MySqlConnector/MySqlCommand.cs b/src/MySqlConnector/MySqlCommand.cs
@@ -363,16 +363,20 @@ public override ValueTask DisposeAsync()
 
 		void ICancellableCommand.SetTimeout(int milliseconds)
 		{
+			Volatile.Write(ref m_commandTimedOut, false);
+
 			if (m_cancelTimerId != 0)
 				TimerQueue.Instance.Remove(m_cancelTimerId);
 
 			if (milliseconds != Constants.InfiniteTimeout)
 			{
-				m_cancelAction ??= Cancel;
-				m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelAction);
+				m_cancelForCommandTimeoutAction ??= CancelCommandForTimeout;
+				m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelForCommandTimeoutAction);
 			}
 		}
 
+		bool ICancellableCommand.IsTimedOut => Volatile.Read(ref m_commandTimedOut);
+
 		int ICancellableCommand.CommandId => m_commandId;
 
 		int ICancellableCommand.CancelAttemptCount { get; set; }
@@ -381,6 +385,12 @@ void ICancellableCommand.SetTimeout(int milliseconds)
 
 		private IOBehavior AsyncIOBehavior => Connection?.AsyncIOBehavior ?? IOBehavior.Asynchronous;
 
+		private void CancelCommandForTimeout()
+		{
+			Volatile.Write(ref m_commandTimedOut, true);
+			Cancel();
+		}
+
 		private bool IsValid([NotNullWhen(false)] out Exception? exception)
 		{
 			exception = null;
@@ -413,6 +423,8 @@ private bool IsValid([NotNullWhen(false)] out Exception? exception)
 		CommandType m_commandType;
 		CommandBehavior m_commandBehavior;
 		Action? m_cancelAction;
+		Action? m_cancelForCommandTimeoutAction;
 		uint m_cancelTimerId;
+		bool m_commandTimedOut;
 	}
 }
diff --git a/src/MySqlConnector/MySqlDataReader.cs b/src/MySqlConnector/MySqlDataReader.cs
@@ -127,6 +127,9 @@ private void ActivateResultSet(CancellationToken cancellationToken)
 				if (mySqlException?.ErrorCode == MySqlErrorCode.QueryInterrupted && cancellationToken.IsCancellationRequested)
 					throw new OperationCanceledException(mySqlException.Message, mySqlException, cancellationToken);
 
+				if (mySqlException?.ErrorCode == MySqlErrorCode.QueryInterrupted && Command!.CancellableCommand.IsTimedOut)
+					throw MySqlException.CreateForTimeout(mySqlException);
+
 				if (mySqlException is not null)
 					throw new MySqlException(mySqlException.ErrorCode, mySqlException.SqlState, mySqlException.Message, mySqlException);
 
diff --git a/tests/MySqlConnector.Tests/CancellationTests.cs b/tests/MySqlConnector.Tests/CancellationTests.cs
@@ -40,7 +40,9 @@ public void Test(int step, int method)
 				var stopwatch = Stopwatch.StartNew();
 				var ex = Assert.Throws<MySqlException>(() => s_executeMethods[method](command));
 				Assert.InRange(stopwatch.ElapsedMilliseconds, 900, 1500);
-				Assert.Equal(MySqlErrorCode.QueryInterrupted, ex.ErrorCode);
+				Assert.Equal(MySqlErrorCode.CommandTimeoutExpired, ex.ErrorCode);
+				var inner = Assert.IsType<MySqlException>(ex.InnerException);
+				Assert.Equal(MySqlErrorCode.QueryInterrupted, inner.ErrorCode);
 
 				// connection should still be usable
 				Assert.Equal(ConnectionState.Open, connection.State);
@@ -63,7 +65,9 @@ public async Task Test(int step, int method)
 				var stopwatch = Stopwatch.StartNew();
 				var ex = await Assert.ThrowsAsync<MySqlException>(async () => await s_executeAsyncMethods[method](command, default));
 				Assert.InRange(stopwatch.ElapsedMilliseconds, 900, 1500);
-				Assert.Equal(MySqlErrorCode.QueryInterrupted, ex.ErrorCode);
+				Assert.Equal(MySqlErrorCode.CommandTimeoutExpired, ex.ErrorCode);
+				var inner = Assert.IsType<MySqlException>(ex.InnerException);
+				Assert.Equal(MySqlErrorCode.QueryInterrupted, inner.ErrorCode);
 			}
 		}
 
@@ -147,7 +151,9 @@ public void Timeout(int method)
 				var stopwatch = Stopwatch.StartNew();
 				var ex = Assert.Throws<MySqlException>(() => s_executeMethods[method](command));
 				Assert.InRange(stopwatch.ElapsedMilliseconds, 900, 1500);
-				Assert.Equal(MySqlErrorCode.QueryInterrupted, ex.ErrorCode);
+				Assert.Equal(MySqlErrorCode.CommandTimeoutExpired, ex.ErrorCode);
+				var inner = Assert.IsType<MySqlException>(ex.InnerException);
+				Assert.Equal(MySqlErrorCode.QueryInterrupted, inner.ErrorCode);
 			}
 
 			[SkipCITheory]
@@ -181,7 +187,9 @@ public async Task Timeout(int method)
 				var stopwatch = Stopwatch.StartNew();
 				var ex = await Assert.ThrowsAsync<MySqlException>(async () => await s_executeAsyncMethods[method](command, default));
 				Assert.InRange(stopwatch.ElapsedMilliseconds, 900, 1500);
-				Assert.Equal(MySqlErrorCode.QueryInterrupted, ex.ErrorCode);
+				Assert.Equal(MySqlErrorCode.CommandTimeoutExpired, ex.ErrorCode);
+				var inner = Assert.IsType<MySqlException>(ex.InnerException);
+				Assert.Equal(MySqlErrorCode.QueryInterrupted, inner.ErrorCode);
 			}
 
 			[SkipCITheory]

Original file line number	Diff line number	Diff line change
`@@ -15,6 +15,7 @@ internal interface ICancellableCommand`
`15`	`15`	`MySqlConnection? Connection { get; }`
`16`	`16`	`IDisposable? RegisterCancel(CancellationToken cancellationToken);`
`17`	`17`	`void SetTimeout(int milliseconds);`
	`18`	`+ bool IsTimedOut { get; }`
`18`	`19`	`}`
`19`	`20`
`20`	`21`	`internal static class ICancellableCommandExtensions`
Original file line number	Diff line number	Diff line change
`@@ -248,6 +248,8 @@ public async Task<bool> ReadAsync(IOBehavior ioBehavior, CancellationToken cance`
`248`	`248`	`resultSet.BufferState = resultSet.State = ResultSetState.NoMoreData;`
`249`	`249`	`if (ex.ErrorCode == MySqlErrorCode.QueryInterrupted && token.IsCancellationRequested)`
`250`	`250`	`throw new OperationCanceledException(ex.Message, ex, token);`
	`251`	`+ if (ex.ErrorCode == MySqlErrorCode.QueryInterrupted && resultSet.Command.CancellableCommand.IsTimedOut)`
	`252`	`+ throw MySqlException.CreateForTimeout(ex);`
`251`	`253`	`throw;`
`252`	`254`	`}`
`253`	`255`	`return ScanRowAsyncRemainder(resultSet, payloadData, row);`
Original file line number	Diff line number	Diff line change
`@@ -173,16 +173,26 @@ public void Dispose()`
`173`	`173`
`174`	`174`	`void ICancellableCommand.SetTimeout(int milliseconds)`
`175`	`175`	`{`
	`176`	`+ Volatile.Write(ref m_commandTimedOut, false);`
	`177`	`+`
`176`	`178`	`if (m_cancelTimerId != 0)`
`177`	`179`	`TimerQueue.Instance.Remove(m_cancelTimerId);`
`178`	`180`
`179`	`181`	`if (milliseconds != Constants.InfiniteTimeout)`
`180`	`182`	`{`
`181`		`- m_cancelAction ??= Cancel;`
`182`		`- m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelAction);`
	`183`	`+ m_cancelForCommandTimeoutAction ??= CancelCommandForTimeout;`
	`184`	`+ m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelForCommandTimeoutAction);`
`183`	`185`	`}`
`184`	`186`	`}`
`185`	`187`
	`188`	`+ bool ICancellableCommand.IsTimedOut => Volatile.Read(ref m_commandTimedOut);`
	`189`	`+`
	`190`	`+ private void CancelCommandForTimeout()`
	`191`	`+ {`
	`192`	`+ Volatile.Write(ref m_commandTimedOut, true);`
	`193`	`+ Cancel();`
	`194`	`+ }`
	`195`	`+`
`186`	`196`	`private async Task<int> ExecuteNonQueryAsync(IOBehavior ioBehavior, CancellationToken cancellationToken)`
`187`	`197`	`{`
`188`	`198`	`((ICancellableCommand) this).ResetCommandTimeout();`
`@@ -309,6 +319,8 @@ private bool IsPrepared`
`309`	`319`	`readonly int m_commandId;`
`310`	`320`	`bool m_isDisposed;`
`311`	`321`	`Action? m_cancelAction;`
	`322`	`+ Action? m_cancelForCommandTimeoutAction;`
`312`	`323`	`uint m_cancelTimerId;`
	`324`	`+ bool m_commandTimedOut;`
`313`	`325`	`}`
`314`	`326`	`}`
Original file line number	Diff line number	Diff line change
`@@ -363,16 +363,20 @@ public override ValueTask DisposeAsync()`
`363`	`363`
`364`	`364`	`void ICancellableCommand.SetTimeout(int milliseconds)`
`365`	`365`	`{`
	`366`	`+ Volatile.Write(ref m_commandTimedOut, false);`
	`367`	`+`
`366`	`368`	`if (m_cancelTimerId != 0)`
`367`	`369`	`TimerQueue.Instance.Remove(m_cancelTimerId);`
`368`	`370`
`369`	`371`	`if (milliseconds != Constants.InfiniteTimeout)`
`370`	`372`	`{`
`371`		`- m_cancelAction ??= Cancel;`
`372`		`- m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelAction);`
	`373`	`+ m_cancelForCommandTimeoutAction ??= CancelCommandForTimeout;`
	`374`	`+ m_cancelTimerId = TimerQueue.Instance.Add(milliseconds, m_cancelForCommandTimeoutAction);`
`373`	`375`	`}`
`374`	`376`	`}`
`375`	`377`
	`378`	`+ bool ICancellableCommand.IsTimedOut => Volatile.Read(ref m_commandTimedOut);`
	`379`	`+`
`376`	`380`	`int ICancellableCommand.CommandId => m_commandId;`
`377`	`381`
`378`	`382`	`int ICancellableCommand.CancelAttemptCount { get; set; }`
`@@ -381,6 +385,12 @@ void ICancellableCommand.SetTimeout(int milliseconds)`
`381`	`385`
`382`	`386`	`private IOBehavior AsyncIOBehavior => Connection?.AsyncIOBehavior ?? IOBehavior.Asynchronous;`
`383`	`387`
	`388`	`+ private void CancelCommandForTimeout()`
	`389`	`+ {`
	`390`	`+ Volatile.Write(ref m_commandTimedOut, true);`
	`391`	`+ Cancel();`
	`392`	`+ }`
	`393`	`+`
`384`	`394`	`private bool IsValid([NotNullWhen(false)] out Exception? exception)`
`385`	`395`	`{`
`386`	`396`	`exception = null;`
`@@ -413,6 +423,8 @@ private bool IsValid([NotNullWhen(false)] out Exception? exception)`
`413`	`423`	`CommandType m_commandType;`
`414`	`424`	`CommandBehavior m_commandBehavior;`
`415`	`425`	`Action? m_cancelAction;`
	`426`	`+ Action? m_cancelForCommandTimeoutAction;`
`416`	`427`	`uint m_cancelTimerId;`
	`428`	`+ bool m_commandTimedOut;`
`417`	`429`	`}`
`418`	`430`	`}`