diff --git a/src/NHibernate.Test/CacheTest/FilterKeyFixture.cs b/src/NHibernate.Test/CacheTest/FilterKeyFixture.cs index 3ba1274c68a..77ecf922ee1 100644 --- a/src/NHibernate.Test/CacheTest/FilterKeyFixture.cs +++ b/src/NHibernate.Test/CacheTest/FilterKeyFixture.cs @@ -20,13 +20,13 @@ public void ToStringIncludeAll() var f = new FilterImpl(Sfi.GetFilterDefinition(filterName)); f.SetParameter("pLike", "so%"); var fk = new FilterKey(f); - Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike{'pLike'='so%'}]"), "Like"); + Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionLike['[pLike, so%]']]"), "Like"); filterName = "DescriptionEqualAndValueGT"; f = new FilterImpl(Sfi.GetFilterDefinition(filterName)); f.SetParameter("pDesc", "something").SetParameter("pValue", 10); fk = new FilterKey(f); - Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT{'pDesc'='something', 'pValue'='10'}]"), "Value"); + Assert.That(fk.ToString(), Is.EqualTo("FilterKey[DescriptionEqualAndValueGT['[pDesc, something]', '[pValue, 10]']]"), "Value"); } [Test] diff --git a/src/NHibernate.Test/CacheTest/QueryKeyFixture.cs b/src/NHibernate.Test/CacheTest/QueryKeyFixture.cs index cbd6870ccd6..dd62799eb28 100644 --- a/src/NHibernate.Test/CacheTest/QueryKeyFixture.cs +++ b/src/NHibernate.Test/CacheTest/QueryKeyFixture.cs @@ -1,9 +1,9 @@ -using System.Collections; using System.Collections.Generic; using NHibernate.Cache; using NHibernate.Engine; using NHibernate.Impl; using NHibernate.SqlCommand; +using NHibernate.Util; using NUnit.Framework; namespace NHibernate.Test.CacheTest @@ -88,7 +88,7 @@ public void HashCodeWithFilters() public void NotEqualHashCodeWithFilters() { // GetHashCode semantic does not guarantee no collision may ever occur, but the algorithm should - // generates different hashcodes for similar but inequal cases. These tests check that cache keys + // generates different hashcodes for similar but unequal cases. These tests check that cache keys // for a query generated for different parameters values are no more equal. QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, false); Assert.That(qk.GetHashCode(), Is.Not.EqualTo(qk1.GetHashCode()), "qk & qk1"); @@ -100,6 +100,20 @@ public void NotEqualHashCodeWithFilters() Assert.That(qk1.GetHashCode(), Is.Not.EqualTo(qvk1.GetHashCode()), "qk1 & qvk1"); } + [Test] + public void HashCodeWithFiltersAndSerialization() + { + QueryKeyFilterDescLikeToCompare(out var qk, out var qk1, true); + var bytes = SerializationHelper.Serialize(qk); + qk = (QueryKey) SerializationHelper.Deserialize(bytes); + Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Like"); + + QueryKeyFilterDescValueToCompare(out qk, out qk1, true); + bytes = SerializationHelper.Serialize(qk); + qk = (QueryKey) SerializationHelper.Deserialize(bytes); + Assert.That(qk.GetHashCode(), Is.EqualTo(qk1.GetHashCode()), "Value"); + } + [Test] public void ToStringWithFilters() { diff --git a/src/NHibernate.Test/DebugSessionFactory.cs b/src/NHibernate.Test/DebugSessionFactory.cs index 152105d473a..eb43b825eee 100644 --- a/src/NHibernate.Test/DebugSessionFactory.cs +++ b/src/NHibernate.Test/DebugSessionFactory.cs @@ -30,25 +30,31 @@ namespace NHibernate.Test /// it is used when testing to check that tests clean up after themselves. /// /// Sessions opened from other sessions are not tracked. + [Serializable] public partial class DebugSessionFactory : ISessionFactoryImplementor { + [NonSerialized] + private DebugConnectionProvider _debugConnectionProvider; + /// /// The debug connection provider if configured for using it, otherwise. /// Use ActualFactory.ConnectionProvider if needing unconditionally the connection provider, be /// it debug or not. /// - public DebugConnectionProvider DebugConnectionProvider { get; } + public DebugConnectionProvider DebugConnectionProvider + => _debugConnectionProvider ?? + (_debugConnectionProvider = ActualFactory.ConnectionProvider as DebugConnectionProvider); public ISessionFactoryImplementor ActualFactory { get; } public EventListeners EventListeners => ((SessionFactoryImpl)ActualFactory).EventListeners; + [NonSerialized] private readonly ConcurrentBag _openedSessions = new ConcurrentBag(); private static readonly ILog _log = LogManager.GetLogger(typeof(DebugSessionFactory).Assembly, typeof(TestCase)); public DebugSessionFactory(ISessionFactory actualFactory) { ActualFactory = (ISessionFactoryImplementor)actualFactory; - DebugConnectionProvider = ActualFactory.ConnectionProvider as DebugConnectionProvider; } #region Session tracking diff --git a/src/NHibernate/Cache/CacheKey.cs b/src/NHibernate/Cache/CacheKey.cs index 87bc96402e5..3b0a46d7266 100644 --- a/src/NHibernate/Cache/CacheKey.cs +++ b/src/NHibernate/Cache/CacheKey.cs @@ -1,4 +1,5 @@ using System; +using System.Runtime.Serialization; using NHibernate.Engine; using NHibernate.Type; @@ -10,12 +11,15 @@ namespace NHibernate.Cache /// keys which do not properly implement equals()/hashCode(). /// [Serializable] - public class CacheKey + public class CacheKey : IDeserializationCallback { private readonly object key; private readonly IType type; private readonly string entityOrRoleName; - private readonly int hashCode; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; + private readonly ISessionFactoryImplementor _factory; /// /// Construct a new key for a collection or entity instance. @@ -31,7 +35,9 @@ public CacheKey(object id, IType type, string entityOrRoleName, ISessionFactoryI key = id; this.type = type; this.entityOrRoleName = entityOrRoleName; - hashCode = type.GetHashCode(key, factory); + _factory = factory; + + _hashCode = GenerateHashCode(); } //Mainly for SysCache and Memcache @@ -50,7 +56,22 @@ public override bool Equals(object obj) public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); + } + + private int GenerateHashCode() + { + return type.GetHashCode(key, _factory); } public object Key @@ -63,4 +84,4 @@ public string EntityOrRoleName get { return entityOrRoleName; } } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Cache/FilterKey.cs b/src/NHibernate/Cache/FilterKey.cs index 0547712044a..58c9ccd9d0f 100644 --- a/src/NHibernate/Cache/FilterKey.cs +++ b/src/NHibernate/Cache/FilterKey.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using NHibernate.Engine; using NHibernate.Impl; using NHibernate.Type; @@ -11,29 +12,38 @@ namespace NHibernate.Cache public class FilterKey { private readonly string _filterName; - private readonly Dictionary _filterParameters = new Dictionary(); + + // Sets and dictionaries are populated last during deserialization, causing them to be potentially empty + // during the deserialization callback. This causes them to be unreliable when used in hashcode or equals + // computations. These computations occur during the deserialization callback for example when another + // serialized set or dictionary contain an instance of this class. (QueryKey was having such a dictionary + // or set.) + // So better serialize them as other structures, so long for Equals implementation which actually needs a + // dictionary. + private readonly KeyValuePair[] _filterParameters; // Since v5.2 [Obsolete("Use overload taking a FilterImpl")] public FilterKey(string name, IEnumerable> @params, IDictionary types) { _filterName = name; - foreach (KeyValuePair me in @params) - { - IType type = types[me.Key]; - _filterParameters[me.Key] = new TypedValue(type, me.Value); - } + _filterParameters = @params.Select( + p => new KeyValuePair( + p.Key, + new TypedValue(types[p.Key], p.Value))).ToArray(); } public FilterKey(FilterImpl filter) { var types = filter.FilterDefinition.ParameterTypes; _filterName = filter.Name; - foreach (var me in filter.Parameters) - { - var type = types[me.Key]; - _filterParameters[me.Key] = new TypedValue(type, me.Value, filter.GetParameterSpan(me.Key) != null); - } + _filterParameters = filter.Parameters.Select( + p => new KeyValuePair( + p.Key, + new TypedValue( + types[p.Key], + p.Value, + filter.GetParameterSpan(p.Key) != null))).ToArray(); } public override int GetHashCode() @@ -47,15 +57,16 @@ public override int GetHashCode() public override bool Equals(object other) { var that = other as FilterKey; - if (that == null) - { - return false; - } - if (!that._filterName.Equals(_filterName)) + if (that == null || !that._filterName.Equals(_filterName)) return false; - if (!CollectionHelper.DictionaryEquals(that._filterParameters, _filterParameters)) - return false; - return true; + + // BagEquals is less efficient than a DictionaryEquals, but serializing dictionaries causes issues on + // deserialization if GetHashCode or Equals are called in its deserialization callback. And building + // dictionaries on the fly will in most cases be worst than BagEquals, unless re-coding its short-circuits. + return CollectionHelper.BagEquals( + _filterParameters, + that._filterParameters, + NamedParameterComparer.Instance); } public override string ToString() diff --git a/src/NHibernate/Cache/QueryKey.cs b/src/NHibernate/Cache/QueryKey.cs index d9c10662648..cdd8ed44179 100644 --- a/src/NHibernate/Cache/QueryKey.cs +++ b/src/NHibernate/Cache/QueryKey.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Runtime.Serialization; using System.Text; using NHibernate.Engine; using NHibernate.Impl; @@ -11,7 +13,7 @@ namespace NHibernate.Cache { [Serializable] - public class QueryKey + public class QueryKey : IDeserializationCallback { private readonly ISessionFactoryImplementor _factory; private readonly SqlString _sqlQueryString; @@ -19,15 +21,24 @@ public class QueryKey private readonly object[] _values; private readonly int _firstRow = RowSelection.NoValue; private readonly int _maxRows = RowSelection.NoValue; - private readonly IDictionary _namedParameters; - private readonly ISet _filters; + + // Sets and dictionaries are populated last during deserialization, causing them to be potentially empty + // during the deserialization callback. This causes them to be unreliable when used in hashcode or equals + // computations. These computations occur during the deserialization callback for example when another + // serialized set or dictionary contain an instance of this class. + // So better serialize them as other structures, so long for Equals implementation which actually needs a + // dictionary and set. + private readonly KeyValuePair[] _namedParameters; + private readonly FilterKey[] _filters; + private readonly CacheableResultTransformer _customTransformer; - private readonly int _hashCode; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; private int[] _multiQueriesFirstRows; private int[] _multiQueriesMaxRows; - /// /// Initializes a new instance of the class. /// @@ -55,9 +66,11 @@ public QueryKey(ISessionFactoryImplementor factory, SqlString queryString, Query _firstRow = RowSelection.NoValue; _maxRows = RowSelection.NoValue; } - _namedParameters = queryParameters.NamedParameters; - _filters = filters; + + _namedParameters = queryParameters.NamedParameters?.ToArray(); + _filters = filters?.ToArray(); _customTransformer = customTransformer; + _hashCode = ComputeHashCode(); } @@ -127,15 +140,14 @@ public override bool Equals(object other) } } - if (!CollectionHelper.SetEquals(_filters, that._filters)) - { + // BagEquals is less efficient than a SetEquals or DictionaryEquals, but serializing dictionaries causes + // issues on deserialization if GetHashCode or Equals are called in its deserialization callback. And + // building sets or dictionaries on the fly will in most cases be worst than BagEquals, unless re-coding + // its short-circuits. + if (!CollectionHelper.BagEquals(_filters, that._filters)) return false; - } - - if (!CollectionHelper.DictionaryEquals(_namedParameters, that._namedParameters)) - { + if (!CollectionHelper.BagEquals(_namedParameters, that._namedParameters, NamedParameterComparer.Instance)) return false; - } if (!CollectionHelper.SequenceEquals(_multiQueriesFirstRows, that._multiQueriesFirstRows)) { @@ -150,7 +162,17 @@ public override bool Equals(object other) public override int GetHashCode() { - return _hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? ComputeHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = ComputeHashCode(); } public int ComputeHashCode() @@ -161,7 +183,9 @@ public int ComputeHashCode() result = 37 * result + _firstRow.GetHashCode(); result = 37 * result + _maxRows.GetHashCode(); - result = 37 * result + (_namedParameters == null ? 0 : CollectionHelper.GetHashCode(_namedParameters, NamedParameterComparer.Instance)); + result = 37 * result + (_namedParameters == null + ? 0 + : CollectionHelper.GetHashCode(_namedParameters, NamedParameterComparer.Instance)); for (int i = 0; i < _types.Length; i++) { @@ -190,10 +214,7 @@ public int ComputeHashCode() if (_filters != null) { - foreach (object filter in _filters) - { - result = 37 * result + filter.GetHashCode(); - } + result = 37 * result + CollectionHelper.GetHashCode(_filters); } result = 37 * result + (_customTransformer == null ? 0 : _customTransformer.GetHashCode()); diff --git a/src/NHibernate/Engine/AssociationKey.cs b/src/NHibernate/Engine/AssociationKey.cs index d041228de40..cfff45045ae 100644 --- a/src/NHibernate/Engine/AssociationKey.cs +++ b/src/NHibernate/Engine/AssociationKey.cs @@ -12,13 +12,11 @@ internal sealed class AssociationKey { private readonly EntityKey ownerKey; private readonly string propertyName; - private readonly int hashCode; public AssociationKey(EntityKey ownerKey, string propertyName) { this.ownerKey = ownerKey; this.propertyName = propertyName; - hashCode = ownerKey.GetHashCode() ^ propertyName.GetHashCode() ^ ownerKey.EntityName.GetHashCode(); } public override bool Equals(object that) @@ -40,7 +38,7 @@ public override bool Equals(object that) public override int GetHashCode() { - return hashCode; + return ownerKey.GetHashCode() ^ propertyName.GetHashCode() ^ ownerKey.EntityName.GetHashCode(); } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Engine/CollectionKey.cs b/src/NHibernate/Engine/CollectionKey.cs index 28a33c2d379..2a54277dc7c 100644 --- a/src/NHibernate/Engine/CollectionKey.cs +++ b/src/NHibernate/Engine/CollectionKey.cs @@ -1,5 +1,5 @@ using System; -using NHibernate.Engine; +using System.Runtime.Serialization; using NHibernate.Impl; using NHibernate.Persister.Collection; using NHibernate.Type; @@ -10,13 +10,15 @@ namespace NHibernate.Engine /// Uniquely identifies a collection instance in a particular session. /// [Serializable] - public sealed class CollectionKey + public sealed class CollectionKey : IDeserializationCallback { private readonly string role; private readonly object key; private readonly IType keyType; - [NonSerialized] private readonly ISessionFactoryImplementor factory; - private readonly int hashCode; + private readonly ISessionFactoryImplementor factory; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; public CollectionKey(ICollectionPersister persister, object key) : this(persister.Role, key, persister.KeyType, persister.Factory) @@ -29,7 +31,8 @@ private CollectionKey(string role, object key, IType keyType, ISessionFactoryImp this.key = key; this.keyType = keyType; this.factory = factory; - hashCode = GenerateHashCode(); //cache the hashcode + + _hashCode = GenerateHashCode(); } public override bool Equals(object obj) @@ -40,7 +43,17 @@ public override bool Equals(object obj) public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); } private int GenerateHashCode() @@ -69,4 +82,4 @@ public override string ToString() return "CollectionKey" + MessageHelper.CollectionInfoString(factory.GetCollectionPersister(role), key, factory); } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Engine/EntityKey.cs b/src/NHibernate/Engine/EntityKey.cs index 041ee9c80dc..22dde296645 100644 --- a/src/NHibernate/Engine/EntityKey.cs +++ b/src/NHibernate/Engine/EntityKey.cs @@ -1,4 +1,5 @@ using System; +using System.Runtime.Serialization; using NHibernate.Impl; using NHibernate.Persister.Entity; using NHibernate.Type; @@ -10,7 +11,7 @@ namespace NHibernate.Engine /// and the identifier space (eg. tablename) /// [Serializable] - public sealed class EntityKey + public sealed class EntityKey : IDeserializationCallback { private readonly object identifier; private readonly string rootEntityName; @@ -18,9 +19,10 @@ public sealed class EntityKey private readonly IType identifierType; private readonly bool isBatchLoadable; - [NonSerialized] private ISessionFactoryImplementor factory; - private int hashCode; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; /// Construct a unique identifier for an entity class instance public EntityKey(object id, IEntityPersister persister) @@ -44,7 +46,8 @@ private EntityKey(object identifier, string rootEntityName, string entityName, I this.identifierType = identifierType; isBatchLoadable = batchLoadable; this.factory = factory; - hashCode = GenerateHashCode(); + + _hashCode = GenerateHashCode(); } public bool IsBatchLoadable @@ -77,6 +80,21 @@ public override bool Equals(object other) && identifierType.IsEqual(otherKey.Identifier, Identifier, factory); } + public override int GetHashCode() + { + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); + } + private int GenerateHashCode() { int result = 17; @@ -88,24 +106,9 @@ private int GenerateHashCode() return result; } - public override int GetHashCode() - { - return hashCode; - } - public override string ToString() { return "EntityKey" + MessageHelper.InfoString(factory.GetEntityPersister(EntityName), Identifier, factory); } - - /// - /// To use in deserialization callback - /// - /// - internal void SetSessionFactory(ISessionFactoryImplementor sessionFactory) - { - factory = sessionFactory; - hashCode = GetHashCode(); - } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Engine/EntityUniqueKey.cs b/src/NHibernate/Engine/EntityUniqueKey.cs index c5a26b03de2..aa728f37667 100644 --- a/src/NHibernate/Engine/EntityUniqueKey.cs +++ b/src/NHibernate/Engine/EntityUniqueKey.cs @@ -1,4 +1,5 @@ using System; +using System.Runtime.Serialization; using NHibernate.Impl; using NHibernate.Type; @@ -12,13 +13,16 @@ namespace NHibernate.Engine /// /// [Serializable] - public class EntityUniqueKey + public class EntityUniqueKey : IDeserializationCallback { private readonly string entityName; private readonly string uniqueKeyName; private readonly object key; private readonly IType keyType; - private readonly int hashCode; + private readonly ISessionFactoryImplementor _factory; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; // 6.0 TODO: rename keyType as semiResolvedKeyType. That is not the responsibility of this class to make any // assumption on the key type being semi-resolved or not, that is the responsibility of its callers. @@ -37,19 +41,10 @@ public EntityUniqueKey(string entityName, string uniqueKeyName, object semiResol this.uniqueKeyName = uniqueKeyName; key = semiResolvedKey; this.keyType = keyType; - hashCode = GenerateHashCode(factory); - } + _factory = factory; - public int GenerateHashCode(ISessionFactoryImplementor factory) - { - int result = 17; - unchecked - { - result = 37 * result + entityName.GetHashCode(); - result = 37 * result + uniqueKeyName.GetHashCode(); - result = 37 * result + keyType.GetHashCode(key, factory); - } - return result; + // No need to delay computation here, but we need the lazy for the deserialization case. + _hashCode = GenerateHashCode(); } public string EntityName @@ -69,7 +64,29 @@ public string UniqueKeyName public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); + } + + public int GenerateHashCode() + { + int result = 17; + unchecked + { + result = 37 * result + entityName.GetHashCode(); + result = 37 * result + uniqueKeyName.GetHashCode(); + result = 37 * result + keyType.GetHashCode(key, _factory); + } + return result; } public override bool Equals(object obj) diff --git a/src/NHibernate/Engine/Query/QueryPlanCache.cs b/src/NHibernate/Engine/Query/QueryPlanCache.cs index 55e9af47fb5..6a0c0b6a7ee 100644 --- a/src/NHibernate/Engine/Query/QueryPlanCache.cs +++ b/src/NHibernate/Engine/Query/QueryPlanCache.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Runtime.Serialization; using NHibernate.Engine.Query.Sql; using NHibernate.Hql; using NHibernate.Linq; @@ -178,12 +180,22 @@ private ParameterMetadata BuildNativeSQLParameterMetadata(string sqlString) } [Serializable] - private class HQLQueryPlanKey : IEquatable + private class HQLQueryPlanKey : IEquatable, IDeserializationCallback { private readonly string query; private readonly bool shallow; - private readonly HashSet filterNames; - private readonly int hashCode; + + // Sets and dictionaries are populated last during deserialization, causing them to be potentially empty + // during the deserialization callback. This causes them to be unreliable when used in hashcode or equals + // computations. These computations occur during the deserialization callback for example when another + // serialized set or dictionary contain an instance of this class. + // So better serialize them as other structures, so long for Equals implementation which actually needs a + // set. + private readonly string[] _filterNames; + + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; private readonly System.Type queryTypeDiscriminator; public HQLQueryPlanKey(string query, bool shallow, IDictionary enabledFilters) @@ -202,23 +214,16 @@ protected HQLQueryPlanKey(System.Type queryTypeDiscriminator, string query, bool this.query = query; this.shallow = shallow; - if (enabledFilters == null || (enabledFilters.Count == 0)) + if (enabledFilters == null || enabledFilters.Count == 0) { - filterNames = new HashSet(); + _filterNames = Array.Empty(); } else { - filterNames = new HashSet(enabledFilters.Keys); + _filterNames = enabledFilters.Keys.ToArray(); } - unchecked - { - var hash = query.GetHashCode(); - hash = 29 * hash + (shallow ? 1 : 0); - hash = 29 * hash + CollectionHelper.GetHashCode(filterNames, filterNames.Comparer); - hash = 29 * hash + queryTypeDiscriminator.GetHashCode(); - hashCode = hash; - } + _hashCode = GenerateHashCode(); } public override bool Equals(object obj) @@ -238,10 +243,12 @@ public bool Equals(HQLQueryPlanKey that) return false; } - if (!filterNames.SetEquals(that.filterNames)) - { + // BagEquals is less efficient than a SetEquals, but serializing dictionaries causes + // issues on deserialization if GetHashCode or Equals are called in its deserialization callback. And + // building sets on the fly will in most cases be worst than BagEquals, unless re-coding + // its short-circuits. + if (!CollectionHelper.BagEquals(_filterNames, that._filterNames)) return false; - } if (!query.Equals(that.query)) { @@ -258,18 +265,50 @@ public bool Equals(HQLQueryPlanKey that) public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); + } + + private int GenerateHashCode() + { + unchecked + { + var hash = query.GetHashCode(); + hash = 29 * hash + (shallow ? 1 : 0); + hash = 29 * hash + CollectionHelper.GetHashCode(_filterNames); + hash = 29 * hash + queryTypeDiscriminator.GetHashCode(); + return hash; + } } } [Serializable] - private class FilterQueryPlanKey + private class FilterQueryPlanKey : IDeserializationCallback { private readonly string query; private readonly string collectionRole; private readonly bool shallow; - private readonly HashSet filterNames; - private readonly int hashCode; + + // Sets and dictionaries are populated last during deserialization, causing them to be potentially empty + // during the deserialization callback. This causes them to be unreliable when used in hashcode or equals + // computations. These computations occur during the deserialization callback for example when another + // serialized set or dictionary contain an instance of this class. + // So better serialize them as other structures, so long for Equals implementation which actually needs a + // set. + private readonly string[] _filterNames; + + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; public FilterQueryPlanKey(string query, string collectionRole, bool shallow, IDictionary enabledFilters) { @@ -277,20 +316,16 @@ public FilterQueryPlanKey(string query, string collectionRole, bool shallow, IDi this.collectionRole = collectionRole; this.shallow = shallow; - if (enabledFilters == null || (enabledFilters.Count == 0)) + if (enabledFilters == null || enabledFilters.Count == 0) { - filterNames = new HashSet(); + _filterNames = Array.Empty(); } else { - filterNames = new HashSet(enabledFilters.Keys); + _filterNames = enabledFilters.Keys.ToArray(); } - int hash = query.GetHashCode(); - hash = 29 * hash + collectionRole.GetHashCode(); - hash = 29 * hash + (shallow ? 1 : 0); - hash = 29 * hash + CollectionHelper.GetHashCode(filterNames, filterNames.Comparer); - hashCode = hash; + _hashCode = GenerateHashCode(); } public override bool Equals(object obj) @@ -308,10 +343,14 @@ public bool Equals(FilterQueryPlanKey that) { return false; } - if (!filterNames.SetEquals(that.filterNames)) - { + + // BagEquals is less efficient than a SetEquals, but serializing dictionaries causes + // issues on deserialization if GetHashCode or Equals are called in its deserialization callback. And + // building sets on the fly will in most cases be worst than BagEquals, unless re-coding + // its short-circuits. + if (!CollectionHelper.BagEquals(_filterNames, that._filterNames)) return false; - } + if (!query.Equals(that.query)) { return false; @@ -326,7 +365,29 @@ public bool Equals(FilterQueryPlanKey that) public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + return _hashCode ?? GenerateHashCode(); + } + + /// + public void OnDeserialization(object sender) + { + _hashCode = GenerateHashCode(); + } + + private int GenerateHashCode() + { + unchecked + { + var hash = query.GetHashCode(); + hash = 29 * hash + collectionRole.GetHashCode(); + hash = 29 * hash + (shallow ? 1 : 0); + hash = 29 * hash + CollectionHelper.GetHashCode(_filterNames); + return hash; + } } } } diff --git a/src/NHibernate/Impl/Printer.cs b/src/NHibernate/Impl/Printer.cs index 9d522e1d0e3..59bd97a0b0a 100644 --- a/src/NHibernate/Impl/Printer.cs +++ b/src/NHibernate/Impl/Printer.cs @@ -1,6 +1,5 @@ -using System.Collections; using System.Collections.Generic; - +using System.Linq; using NHibernate.Engine; using NHibernate.Intercept; using NHibernate.Metadata; @@ -83,6 +82,15 @@ public string ToString(IDictionary namedTypedValues) return CollectionPrinter.ToString(result); } + internal string ToString(IEnumerable> namedTypedValues) + { + return CollectionPrinter.ToString( + namedTypedValues.Select( + ntv => new KeyValuePair( + ntv.Key, + ntv.Value.Type.ToLoggableString(ntv.Value.Value, _factory)))); + } + public void ToString(object[] entities) { if (!log.IsDebugEnabled() || entities.Length == 0) @@ -109,4 +117,4 @@ public Printer(ISessionFactoryImplementor factory) _factory = factory; } } -} \ No newline at end of file +} diff --git a/src/NHibernate/LockMode.cs b/src/NHibernate/LockMode.cs index c87c4bf3d63..ee44f6a15b9 100644 --- a/src/NHibernate/LockMode.cs +++ b/src/NHibernate/LockMode.cs @@ -15,7 +15,6 @@ public sealed class LockMode { private readonly int level; private readonly string name; - private readonly int hashcode; /// /// @@ -26,10 +25,6 @@ private LockMode(int level, string name) { this.level = level; this.name = name; - unchecked - { - hashcode = (level * 37) ^ (name != null ? name.GetHashCode() : 0); - } } /// @@ -126,7 +121,10 @@ public bool Equals(LockMode other) public override int GetHashCode() { - return hashcode; + unchecked + { + return (level * 37) ^ (name?.GetHashCode() ?? 0); + } } } } diff --git a/src/NHibernate/Util/WeakHashtable.cs b/src/NHibernate/Util/WeakHashtable.cs index 70a9ffa634a..4a60cb49afe 100644 --- a/src/NHibernate/Util/WeakHashtable.cs +++ b/src/NHibernate/Util/WeakHashtable.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.Serialization; using NHibernate.DebugHelpers; namespace NHibernate.Util @@ -10,15 +11,19 @@ namespace NHibernate.Util // instead to avoid requiring UnmanagedCode permission. [DebuggerTypeProxy(typeof(DictionaryProxy))] [Serializable] - public class WeakRefWrapper + public class WeakRefWrapper : IDeserializationCallback { private WeakReference reference; - private int hashCode; + // hashcode may vary among processes, they cannot be stored and have to be re-computed after deserialization + [NonSerialized] + private int? _hashCode; public WeakRefWrapper(object target) { reference = new WeakReference(target); - hashCode = target.GetHashCode(); + + // No need to delay computation here, but we need the lazy for the deserialization case. + _hashCode = GenerateHashCode(); } public override bool Equals(object obj) @@ -42,13 +47,35 @@ public override bool Equals(object obj) return false; } - return hashCode == that.hashCode && + return GetHashCode() == that.GetHashCode() && Equals(target, that.Target); } public override int GetHashCode() { - return hashCode; + // If the object is put in a set or dictionary during deserialization, the hashcode will not yet be + // computed. Compute the hashcode on the fly. So long as this happens only during deserialization, there + // will be no thread safety issues. For the hashcode to be always defined after deserialization, the + // deserialization callback is used. + // As the reference may (unlikely) die between the computation here and the deserialization callback, + // better also store the computed on the fly value. + return _hashCode ?? (_hashCode = GenerateHashCode()).Value; + } + + /// + public void OnDeserialization(object sender) + { + // If the value has already been computed for some reasons, keep-it. Otherwise the hashcode may change + // due to the reference having died. + if (!_hashCode.HasValue) + _hashCode = GenerateHashCode(); + } + + private int GenerateHashCode() + { + // Defaulting to the base value is not a trouble, because a dead reference is considered only equal to + // itself. + return reference.Target?.GetHashCode() ?? base.GetHashCode(); } public object Target