Merge pull request #610 from ckadluba/607-exception-auditto-shared-sqlconnection

ckadluba · web-flow · commit 2bf5519aa7e6 · 2025-03-15T22:01:18.000Z
Fixed concurrency issue #607
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,6 @@
+# 8.1.1
+* Fixed concurrency issue #607 in audit sink
+
 # 8.1.0
 * Implemented #542: Column option `ResolveHierarchicalPropertyName` to force non-hierarchical handling
 * Removed unnecessary exception handlers and let Serilog Core do the SelfLog()
diff --git a/serilog-sinks-mssqlserver.sln b/serilog-sinks-mssqlserver.sln
@@ -28,13 +28,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 		Directory.Build.props = Directory.Build.props
 		Directory.Packages.props = Directory.Packages.props
 		.github\ISSUE_TEMPLATE.md = .github\ISSUE_TEMPLATE.md
+		.github\workflows\perftests.yml = .github\workflows\perftests.yml
 		.github\workflows\pr-analysis-codeql.yml = .github\workflows\pr-analysis-codeql.yml
 		.github\workflows\pr-analysis-devskim.yml = .github\workflows\pr-analysis-devskim.yml
 		.github\workflows\pr-validation.yml = .github\workflows\pr-validation.yml
 		README.md = README.md
 		.github\workflows\release.yml = .github\workflows\release.yml
 		RunPerfTests.ps1 = RunPerfTests.ps1
-		.github\workflows\perftests.yml = .github\workflows\perftests.yml
 	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "NetStandardDemoLib", "sample\NetStandardDemo\NetStandardDemoLib\NetStandardDemoLib.csproj", "{8E69E31B-61C7-4175-B886-9C2078FCA477}"
diff --git a/src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/Hybrid/LoggerConfigurationMSSqlServerExtensions.cs b/src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/Hybrid/LoggerConfigurationMSSqlServerExtensions.cs
@@ -1,4 +1,4 @@
-﻿// Copyright 2024 Serilog Contributors
+﻿// Copyright 2025 Serilog Contributors
 // 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/Microsoft.Extensions.Configuration/LoggerConfigurationMSSqlServerExtensions.cs b/src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/Microsoft.Extensions.Configuration/LoggerConfigurationMSSqlServerExtensions.cs
@@ -1,4 +1,4 @@
-﻿// Copyright 2024 Serilog Contributors
+﻿// Copyright 2025 Serilog Contributors
 // 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerAuditSink.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerAuditSink.cs
@@ -1,4 +1,4 @@
-﻿// Copyright 2024 Serilog Contributors
+﻿// Copyright 2025 Serilog Contributors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -30,8 +30,6 @@ public class MSSqlServerAuditSink : ILogEventSink, IDisposable
     {
         private readonly ISqlLogEventWriter _sqlLogEventWriter;
 
-        private bool _disposedValue;
-
         /// <summary>
         /// Construct a sink posting to the specified database.
         ///
@@ -115,15 +113,7 @@ public void Dispose()
         /// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
         protected virtual void Dispose(bool disposing)
         {
-            if (!_disposedValue)
-            {
-                if (disposing)
-                {
-                    _sqlLogEventWriter.Dispose();
-                }
-
-                _disposedValue = true;
-            }
+            // This class needn't to dispose anything. This is just here for sink interface compatibility.
         }
 
         private static void ValidateParameters(MSSqlServerSinkOptions sinkOptions, ColumnOptions columnOptions)
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSink.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSink.cs
@@ -1,4 +1,4 @@
-﻿// Copyright 2024 Serilog Contributors
+﻿// Copyright 2025 Serilog Contributors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -152,7 +152,6 @@ protected virtual void Dispose(bool disposing)
                 if (disposing)
                 {
                     _sqlBulkBatchWriter.Dispose();
-                    _sqlLogEventWriter.Dispose();
                 }
 
                 _disposedValue = true;
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IXmlPropertyFormatter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/IXmlPropertyFormatter.cs
@@ -1,18 +1,4 @@
-﻿// Copyright 2024 Serilog Contributors
-// 
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-// 
-//     http://www.apache.org/licenses/LICENSE-2.0
-// 
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-using Serilog.Events;
+﻿using Serilog.Events;
 
 namespace Serilog.Sinks.MSSqlServer.Output
 {
@@ -21,4 +7,4 @@ internal interface IXmlPropertyFormatter
         string GetValidElementName(string name);
         string Simplify(LogEventPropertyValue value, ColumnOptions.PropertiesColumnOptions options);
     }
-}
+}
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/JsonLogEventFormatter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Output/JsonLogEventFormatter.cs
@@ -1,17 +1,3 @@
-// Copyright 2024 Serilog Contributors
-// 
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-// 
-//     http://www.apache.org/licenses/LICENSE-2.0
-// 
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
 using System;
 using System.Collections.Generic;
 using System.Data;
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlCommandFactory.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlCommandFactory.cs
@@ -4,7 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Platform
 {
     internal interface ISqlCommandFactory
     {
-        ISqlCommandWrapper CreateCommand(ISqlConnectionWrapper sqlConnection);
         ISqlCommandWrapper CreateCommand(string cmdText, ISqlConnectionWrapper sqlConnection);
     }
 }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlLogEventWriter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlLogEventWriter.cs
@@ -1,11 +1,10 @@
-﻿using System;
-using System.Collections.Generic;
+﻿using System.Collections.Generic;
 using System.Threading.Tasks;
 using Serilog.Events;
 
 namespace Serilog.Sinks.MSSqlServer.Platform
 {
-    internal interface ISqlLogEventWriter : IDisposable
+    internal interface ISqlLogEventWriter
     {
         void WriteEvent(LogEvent logEvent);
 
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlBulkBatchWriter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlBulkBatchWriter.cs
@@ -1,9 +1,7 @@
 ﻿using System;
 using System.Collections.Generic;
 using System.Data;
-using System.Linq;
 using System.Threading.Tasks;
-using Serilog.Debugging;
 using Serilog.Events;
 using Serilog.Sinks.MSSqlServer.Output;
 
@@ -45,10 +43,10 @@ public async Task WriteBatch(IEnumerable<LogEvent> events)
             {
                 FillDataTable(events);
 
-                using (var cn = _sqlConnectionFactory.Create())
+                using (var sqlConnection = _sqlConnectionFactory.Create())
                 {
-                    await cn.OpenAsync().ConfigureAwait(false);
-                    using (var copy = cn.CreateSqlBulkCopy(_disableTriggers, _schemaAndTableName))
+                    await sqlConnection.OpenAsync().ConfigureAwait(false);
+                    using (var copy = sqlConnection.CreateSqlBulkCopy(_disableTriggers, _schemaAndTableName))
                     {
                         for (var i = 0; i < _dataTable.Columns.Count; i++)
                         {
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs
@@ -1,17 +1,10 @@
 ﻿using System;
-using System.Data;
 using System.Threading.Tasks;
-using Microsoft.Data.SqlClient;
 
 namespace Serilog.Sinks.MSSqlServer.Platform.SqlClient
 {
     internal interface ISqlCommandWrapper : IDisposable
     {
-        CommandType CommandType { get; set; }
-        string CommandText { get; set; }
-
-        void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper);
-        void ClearParameters();
         void AddParameter(string parameterName, object value);
         int ExecuteNonQuery();
         Task<int> ExecuteNonQueryAsync();
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapper.cs
@@ -10,32 +10,9 @@ internal class SqlCommandWrapper : ISqlCommandWrapper
         private readonly SqlCommand _sqlCommand;
         private bool _disposedValue;
 
-        public SqlCommandWrapper(SqlCommand sqlCommand, SqlConnection sqlConnection)
+        public SqlCommandWrapper(SqlCommand sqlCommand)
         {
             _sqlCommand = sqlCommand ?? throw new ArgumentNullException(nameof(sqlCommand));
-            _sqlCommand.Connection = sqlConnection ?? throw new ArgumentNullException(nameof(sqlConnection));
-        }
-
-        public CommandType CommandType
-        {
-            get => _sqlCommand.CommandType;
-            set => _sqlCommand.CommandType = value;
-        }
-
-        public string CommandText
-        {
-            get => _sqlCommand.CommandText;
-            set => _sqlCommand.CommandText = value;
-        }
-
-        public void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper)
-        {
-            _sqlCommand.Connection = sqlConnectionWrapper.SqlConnection;
-        }
-
-        public void ClearParameters()
-        {
-            _sqlCommand.Parameters.Clear();
         }
 
         public void AddParameter(string parameterName, object value)
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlCommandFactory.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlCommandFactory.cs
@@ -5,16 +5,10 @@ namespace Serilog.Sinks.MSSqlServer.Platform
 {
     internal class SqlCommandFactory : ISqlCommandFactory
     {
-        public ISqlCommandWrapper CreateCommand(ISqlConnectionWrapper sqlConnection)
-        {
-            var sqlCommand = new SqlCommand();
-            return new SqlCommandWrapper(sqlCommand, sqlConnection.SqlConnection);
-        }
-
         public ISqlCommandWrapper CreateCommand(string cmdText, ISqlConnectionWrapper sqlConnection)
         {
-            var sqlCommand = new SqlCommand(cmdText);
-            return new SqlCommandWrapper(sqlCommand, sqlConnection.SqlConnection);
+            var sqlCommand = new SqlCommand(cmdText, sqlConnection.SqlConnection);
+            return new SqlCommandWrapper(sqlCommand);
         }
     }
 }
diff --git a/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs b/src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlInsertStatementWriter.cs
@@ -1,10 +1,8 @@
 ﻿using System;
 using System.Collections.Generic;
-using System.Data;
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
-using Serilog.Debugging;
 using Serilog.Events;
 using Serilog.Sinks.MSSqlServer.Output;
 using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
@@ -20,8 +18,7 @@ internal class SqlInsertStatementWriter : ISqlLogEventWriter
         private readonly ISqlCommandFactory _sqlCommandFactory;
         private readonly ILogEventDataGenerator _logEventDataGenerator;
 
-        private ISqlCommandWrapper _sqlCommand;
-        private bool _disposedValue;
+        private string _sqlCommandText;
 
         public SqlInsertStatementWriter(
             string tableName,
@@ -43,92 +40,68 @@ public SqlInsertStatementWriter(
 
         public async Task WriteEvents(IEnumerable<LogEvent> events)
         {
-            using (var cn = _sqlConnectionFactory.Create())
+            using (var sqlConnection = _sqlConnectionFactory.Create())
             {
-                await cn.OpenAsync().ConfigureAwait(false);
+                await sqlConnection.OpenAsync().ConfigureAwait(false);
 
                 foreach (var logEvent in events)
                 {
                     var fields = _logEventDataGenerator.GetColumnsAndValues(logEvent).ToList();
-                    InitializeSqlCommand(cn, fields);
-
-                    var index = 0;
-                    _sqlCommand.ClearParameters();
-                    foreach (var field in fields)
+                    using (var sqlCommand = InitializeSqlCommand(sqlConnection, fields))
                     {
-                        _sqlCommand.AddParameter(Invariant($"@P{index}"), field.Value);
-                        index++;
+                        await sqlCommand.ExecuteNonQueryAsync().ConfigureAwait(false);
                     }
-
-                    await _sqlCommand.ExecuteNonQueryAsync().ConfigureAwait(false);
                 }
             }
         }
 
-        /// <summary>
-        /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
-        /// </summary>
-        public void Dispose()
+        private ISqlCommandWrapper InitializeSqlCommand(
+            ISqlConnectionWrapper sqlConnection,
+            IEnumerable<KeyValuePair<string, object>> logEventFields)
         {
-            Dispose(disposing: true);
-            GC.SuppressFinalize(this);
-        }
+            InitializeSqlCommandText(logEventFields);
 
-        /// <summary>
-        /// Releases the unmanaged resources used by the Serilog.Sinks.MSSqlServer.Platform.SqlInsertStatementWriter and optionally
-        /// releases the managed resources.
-        /// </summary>
-        /// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
-        protected virtual void Dispose(bool disposing)
-        {
-            if (!_disposedValue)
+            var sqlCommand = _sqlCommandFactory.CreateCommand(_sqlCommandText, sqlConnection);
+            var index = 0;
+            foreach (var field in logEventFields)
             {
-                if (disposing)
-                {
-                    _sqlCommand?.Dispose();
-                }
-
-                _disposedValue = true;
+                sqlCommand.AddParameter(Invariant($"@P{index}"), field.Value);
+                index++;
             }
+
+            return sqlCommand;
         }
 
-        private void InitializeSqlCommand(ISqlConnectionWrapper sqlConnection,
-            IEnumerable<KeyValuePair<string, object>> logEventFields)
+        private void InitializeSqlCommandText(IEnumerable<KeyValuePair<string, object>> logEventFields)
         {
-            // Optimization: generate INSERT statement and SqlCommand only once
-            // and reuse it with different values and SqlConnections because
-            // the structure does not change.
-            if (_sqlCommand == null)
+            if (_sqlCommandText != null)
             {
-                _sqlCommand = _sqlCommandFactory.CreateCommand(sqlConnection);
-                _sqlCommand.CommandType = CommandType.Text;
+                return;
+            }
 
-                var fieldList = new StringBuilder(Invariant($"INSERT INTO [{_schemaName}].[{_tableName}] ("));
-                var parameterList = new StringBuilder(") VALUES (");
+            var fieldList = new StringBuilder(Invariant($"INSERT INTO [{_schemaName}].[{_tableName}] ("));
+            var parameterList = new StringBuilder(") VALUES (");
 
-                var index = 0;
-                foreach (var field in logEventFields)
+            var index = 0;
+            foreach (var field in logEventFields)
+            {
+                if (index != 0)
                 {
-                    if (index != 0)
-                    {
-                        fieldList.Append(',');
-                        parameterList.Append(',');
-                    }
-
-                    fieldList.Append(Invariant($"[{field.Key}]"));
-                    parameterList.Append("@P");
-                    parameterList.Append(index);
-
-                    index++;
+                    fieldList.Append(',');
+                    parameterList.Append(',');
                 }
 
-                parameterList.Append(')');
-                fieldList.Append(parameterList);
+                fieldList.Append(Invariant($"[{field.Key}]"));
+                parameterList.Append("@P");
+                parameterList.Append(index);
 
-                _sqlCommand.CommandText = fieldList.ToString();
+                index++;
             }
 
-            _sqlCommand.SetConnection(sqlConnection);
+            parameterList.Append(')');
+            fieldList.Append(parameterList);
+
+            _sqlCommandText = fieldList.ToString();
         }
     }
 }
diff --git a/test/Serilog.Sinks.MSSqlServer.PerformanceTests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterBenchmarks.cs b/test/Serilog.Sinks.MSSqlServer.PerformanceTests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterBenchmarks.cs
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerAuditSinkTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerAuditSinkTests.cs
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerSinkTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/MSSqlServerSinkTests.cs
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapperTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlClient/SqlCommandWrapperTests.cs
diff --git a/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs b/test/Serilog.Sinks.MSSqlServer.Tests/Sinks/MSSqlServer/Platform/SqlInsertStatementWriterTests.cs

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Copyright 2024 Serilog Contributors`
	`1`	`+// Copyright 2025 Serilog Contributors`
`2`	`2`	`//`
`3`	`3`	`// Licensed under the Apache License, Version 2.0 (the "License");`
`4`	`4`	`// you may not use this file except in compliance with the License.`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Platform`
`4`	`4`	`{`
`5`	`5`	`internal interface ISqlCommandFactory`
`6`	`6`	`{`
`7`		`- ISqlCommandWrapper CreateCommand(ISqlConnectionWrapper sqlConnection);`
`8`	`7`	`ISqlCommandWrapper CreateCommand(string cmdText, ISqlConnectionWrapper sqlConnection);`
`9`	`8`	`}`
`10`	`9`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,10 @@`
`1`		`-using System;`
`2`		`-using System.Collections.Generic;`
	`1`	`+using System.Collections.Generic;`
`3`	`2`	`using System.Threading.Tasks;`
`4`	`3`	`using Serilog.Events;`
`5`	`4`
`6`	`5`	`namespace Serilog.Sinks.MSSqlServer.Platform`
`7`	`6`	`{`
`8`		`- internal interface ISqlLogEventWriter : IDisposable`
	`7`	`+ internal interface ISqlLogEventWriter`
`9`	`8`	`{`
`10`	`9`	`void WriteEvent(LogEvent logEvent);`
`11`	`10`