Merge pull request #1678 from fredericDelaporte/BackPortBitwiseFix4.1

Back port bitwise thread safety fix in 4.1

Author: fredericDelaporte

build-common/common.xml

@@ -52,7 +52,7 @@
 		effectively SP0).
 	-->
 
-	<property name="project.version" value="4.1.1.GA" overwrite="false" />
+	<property name="project.version" value="4.1.2.GA" overwrite="false" />
 
 	<!-- This version number should be changed if, but only if, there are incompatible
 	     changes compared to the previous version. -->

releasenotes.txt

@@ -1,3 +1,11 @@
+Build 4.1.2.GA
+=============================
+
+Release notes - NHibernate - Version 4.1.2.GA
+
+** Bug
+    * #1355 NH-3928 - Random invalid SQL generated when using bitwise operators
+
 Build 4.1.1.GA
 =============================
 

src/NHibernate.Test/Hql/HQLFunctions.cs

@@ -1,5 +1,9 @@
 using System;
 using System.Collections;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Threading;
+using System.Threading.Tasks;
 using NHibernate.Dialect;
 using NUnit.Framework;
 
@@ -21,6 +25,18 @@ static HQLFunctions()
 						{"locate", new[] {typeof (SQLiteDialect)}},
 						{"bit_length", new[] {typeof (SQLiteDialect)}},
 						{"extract", new[] {typeof (SQLiteDialect)}},
+						{
+							"bxor",
+							new[]
+							{
+								// Could be supported like Oracle, with a template
+								typeof (SQLiteDialect),
+								// Could be supported by overriding registration with # instead of ^
+								typeof (PostgreSQLDialect),
+								typeof (PostgreSQL81Dialect),
+								typeof (PostgreSQL82Dialect)
+							}
+						},
 						{"nullif", new[] {typeof (Oracle8iDialect)}}
 					};
 		}
@@ -1014,7 +1030,202 @@ public void ParameterLikeArgument()
 				Assert.AreEqual(1, l.Count);
 			}
 		}
+
+		[Test]
+		public void BitwiseAnd()
+		{
+			IgnoreIfNotSupported("band");
+			CreateMaterialResources();
+
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				var query = s.CreateQuery("from MaterialResource m where (m.State & 1) > 0");
+				var result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "& 1");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State & 2) > 0");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "& 2");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State & 3) > 0");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(2), "& 3");
+
+				tx.Commit();
+			}
+			DeleteMaterialResources();
+		}
+
+		[Test]
+		public void BitwiseOr()
+		{
+			IgnoreIfNotSupported("bor");
+			CreateMaterialResources();
+
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				var query = s.CreateQuery("from MaterialResource m where (m.State | 1) > 0");
+				var result = query.List();
+				Assert.That(result, Has.Count.EqualTo(3), "| 1) > 0");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State | 1) > 1");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "| 1) > 1");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State | 0) > 0");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(2), "| 0) > 0");
+
+				tx.Commit();
+			}
+			DeleteMaterialResources();
+		}
+
+		[Test]
+		public void BitwiseXor()
+		{
+			IgnoreIfNotSupported("bxor");
+			CreateMaterialResources();
+
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				var query = s.CreateQuery("from MaterialResource m where (m.State ^ 1) > 0");
+				var result = query.List();
+				Assert.That(result, Has.Count.EqualTo(2), "^ 1");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State ^ 2) > 0");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(2), "^ 2");
+
+				query = s.CreateQuery("from MaterialResource m where (m.State ^ 3) > 0");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(3), "^ 3");
+
+				tx.Commit();
+			}
+			DeleteMaterialResources();
+		}
+
+		[Test]
+		public void BitwiseNot()
+		{
+			IgnoreIfNotSupported("bnot");
+			IgnoreIfNotSupported("band");
+			CreateMaterialResources();
+
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				// ! takes not precedence over & at least with some dialects (maybe all).
+				var query = s.CreateQuery("from MaterialResource m where ((!m.State) & 3) = 3");
+				var result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "((!m.State) & 3) = 3");
+
+				query = s.CreateQuery("from MaterialResource m where ((!m.State) & 3) = 2");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "((!m.State) & 3) = 2");
+
+				query = s.CreateQuery("from MaterialResource m where ((!m.State) & 3) = 1");
+				result = query.List();
+				Assert.That(result, Has.Count.EqualTo(1), "((!m.State) & 3) = 1");
+
+				tx.Commit();
+			}
+			DeleteMaterialResources();
+		}
+
+		// #1670
+		[Test]
+		public void BitwiseIsThreadsafe()
+		{
+			IgnoreIfNotSupported("band");
+			IgnoreIfNotSupported("bor");
+			IgnoreIfNotSupported("bxor");
+			IgnoreIfNotSupported("bnot");
+			var queries = new List<Tuple<string, int>>
+			{
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State & 1) > 0", 1),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State & 2) > 0", 1),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State & 3) > 0", 2),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State | 1) > 0", 3),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State | 1) > 1", 1),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State | 0) > 0", 2),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State ^ 1) > 0", 2),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State ^ 2) > 0", 2),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where (m.State ^ 3) > 0", 3),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where ((!m.State) & 3) = 3", 1),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where ((!m.State) & 3) = 2", 1),
+				new Tuple<string, int> ("select count(*) from MaterialResource m where ((!m.State) & 3) = 1", 1)
+			};
+			// Do not use a ManualResetEventSlim, it does not support async and exhausts the task thread pool in the
+			// async counterparts of this test. SemaphoreSlim has the async support and release the thread when waiting.
+			var semaphore = new SemaphoreSlim(0);
+			var failures = new ConcurrentBag<Exception>();
+
+			CreateMaterialResources();
+
+			Parallel.For(
+				0, queries.Count + 1,
+				i =>
+				{
+					if (i >= queries.Count)
+					{
+						// Give some time to threads for reaching the wait, having all of them ready to do the
+						// critical part of their job concurrently.
+						Thread.Sleep(100);
+						semaphore.Release(queries.Count);
+						return;
+					}
+
+					try
+					{
+						var query = queries[i];
+						using (var s = OpenSession())
+						using (var tx = s.BeginTransaction())
+						{
+							semaphore.Wait();
+							var q = s.CreateQuery(query.Item1);
+							var result = q.UniqueResult<long>();
+							Assert.That(result, Is.EqualTo(query.Item2), query.Item1);
+							tx.Commit();
+						}
+					}
+					catch (Exception e)
+					{
+						failures.Add(e);
+					}
+				});
+
+			Assert.That(failures, Is.Empty, $"{failures.Count} task(s) failed.");
+			DeleteMaterialResources();
+		}
+
+		private void CreateMaterialResources()
+		{
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				s.Save(new MaterialResource("m1", "18", MaterialResource.MaterialState.Available));
+				s.Save(new MaterialResource("m2", "19", MaterialResource.MaterialState.Reserved));
+				s.Save(new MaterialResource("m3", "20", MaterialResource.MaterialState.Discarded));
+				tx.Commit();
+			}
+		}
+
+		private void DeleteMaterialResources()
+		{
+			using (var s = OpenSession())
+			using (var tx = s.BeginTransaction())
+			{
+				s.CreateQuery("delete from MaterialResource").ExecuteUpdate();
+				tx.Commit();
+			}
+		}
 	}
+
 	public class ForNh1725
 	{
 		public string Description { get; set; }

src/NHibernate.sln.DotSettings

@@ -19,9 +19,11 @@
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateConstants/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateInstanceFields/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
 	<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="AaBb" /&gt;</s:String>
+	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ECSharpAttributeForSingleLineMethodUpgrade/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EAddAccessorOwnerDeclarationBracesMigration/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateBlankLinesAroundFieldToBlankLinesAroundProperty/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002ESettingsUpgrade_002EMigrateThisQualifierSettings/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/Environment/UnitTesting/SeparateAppDomainPerAssembly/@EntryValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=0BE95D01E0F2244E97F5FEFAD1EB1A63/@KeyIndexDefined">True</s:Boolean>
 	<s:String x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=0BE95D01E0F2244E97F5FEFAD1EB1A63/Comment/@EntryValue">NUnit Assert.NotNull</s:String>
 	<s:Boolean x:Key="/Default/PatternsAndTemplates/StructuralSearch/Pattern/=0BE95D01E0F2244E97F5FEFAD1EB1A63/CustomPatternPlaceholder/=actual/@KeyIndexDefined">True</s:Boolean>

src/NHibernate/Dialect/BitwiseFunctionOperation.cs

@@ -14,99 +14,63 @@ namespace NHibernate.Dialect
 	public class BitwiseFunctionOperation : ISQLFunction
 	{
 		private readonly string _functionName;
-		private SqlStringBuilder _sqlBuffer;
-		private Queue _args;
 
 		/// <summary>
-		/// Creates an instance of this class using the provided function name
+		/// Creates an instance of this class using the provided function name.
 		/// </summary>
 		/// <param name="functionName">
-		/// The bitwise function name as defined by the SQL-Dialect
+		/// The bitwise function name as defined by the SQL-Dialect.
 		/// </param>
 		public BitwiseFunctionOperation(string functionName)
 		{
-			_functionName = functionName;			
+			_functionName = functionName;
 		}
 
 		#region ISQLFunction Members
 
+		/// <inheritdoc />
 		public IType ReturnType(IType columnType, IMapping mapping)
 		{
 			return NHibernateUtil.Int64;
 		}
 
-		public bool HasArguments
-		{
-			get { return true; }
-		}
+		/// <inheritdoc />
+		public bool HasArguments => true;
 
-		public bool HasParenthesesIfNoArguments
-		{
-			get { return true; }
-		}
+		/// <inheritdoc />
+		public bool HasParenthesesIfNoArguments => true;
 
+		/// <inheritdoc />
 		public SqlString Render(IList args, ISessionFactoryImplementor factory)
 		{
-			Prepare(args);
-
-			AddFunctionName(); 
-			OpenParens(); 
-			AddArguments(); 
-			CloseParens();
-
-			return SqlResult();
-		}
-
-		#endregion
+			var sqlBuffer = new SqlStringBuilder();
 
-		private void Prepare(IList args)
-		{
-			_sqlBuffer = new SqlStringBuilder();
-			_args = new Queue();
+			sqlBuffer.Add(_functionName);
+			sqlBuffer.Add("(");
 			foreach (var arg in args)
 			{
-				if (!IsParens(arg.ToString()))
-					_args.Enqueue(arg);
-			}
-		}
-
-		private static bool IsParens(string candidate)
-		{
-			return candidate == "(" || candidate == ")";
-		}
-
-		private void AddFunctionName()
-		{
-			_sqlBuffer.Add(_functionName);
-		}
-
-		private void OpenParens()
-		{
-			_sqlBuffer.Add("(");
-		}
-
-		private void AddArguments()
-		{
-			while (_args.Count > 0)
-			{
-				var arg = _args.Dequeue();
+				// The actual second argument may be surrounded by parentesis as additional arguments.
+				// They have to be ignored, otherwise it would emit "functionName(firstArg, (, secondArg, ))"
+				if (IsParens(arg.ToString()))
+					continue;
 				if (arg is Parameter || arg is SqlString)
-					_sqlBuffer.AddObject(arg);
+					sqlBuffer.AddObject(arg);
 				else
-					_sqlBuffer.Add(arg.ToString());
-				if (_args.Count > 0)
-					_sqlBuffer.Add(", ");
+					sqlBuffer.Add(arg.ToString());
+				sqlBuffer.Add(", ");
 			}
-		}
 
-		private void CloseParens()
-		{
-			_sqlBuffer.Add(")");
+			sqlBuffer.RemoveAt(sqlBuffer.Count - 1);
+			sqlBuffer.Add(")");
+
+			return sqlBuffer.ToSqlString();
 		}
 
-		private SqlString SqlResult()
+		#endregion
+
+		private static bool IsParens(string candidate)
 		{
-			return _sqlBuffer.ToSqlString();
+			return candidate == "(" || candidate == ")";
 		}
 	}
 }