Contextual Filter Definition Cache Key Mismatch Error

[gokhanabatay]

#1888

[fredericDelaporte]

They do not have the same semantic at all. SequenceEquals cares about order of elements, not SetEquals. Order of elements is irrelevant here, SequenceEquals must not be used.

If SetEquals only calls GetHashCode in your case and not Equals for elements supposed to be equals, it means there is a flaw in the GetHashCode implementation causing it to not yield the same value for equals elements.

So that is the FilterKey.GetHashCode which would need a fix, not the code here.

Can you please add a test case demonstrating the issue? We would not merge a change without a test case checking something has actually been fixed anyway.

[gokhanabatay]

There is an other issue that i cannot figure it out;
When i write,

ISet<FilterKey> key1 = new HashSet<FilterKey>(){new FilterKey(....)};
ISet<FilterKey> key2 = new HashSet<FilterKey>(){new FilterKey(....)};

bool isEqual = key1.SetEquals(key2);

It is actually equal and returns equal and it calls FilterKey.Equals method in HashSet dot net core code.

But NHibernate QueryKey line 130 same code does not call the Equals method.

If you try to create new SessionFilter and check with cached query you could see what I try to tell you.

And GetHashCode of the _filter and that._filter has different HashCodes.

ContainsAllElements(otherAsSet); called at below code of dot net core

And this Contains method just calls GetHashCode why is that?

        private bool ContainsAllElements(IEnumerable<T> other)
        {
            foreach (T element in other)
            {
                if (!Contains(element))
                {
                    return false;
                }
            }
            return true;
        }

        /// <summary>
        /// Checks if this and other contain the same elements. This is set equality: 
        /// duplicates and order are ignored
        /// </summary>
        /// <param name="other"></param>
        /// <returns></returns>
        public bool SetEquals(IEnumerable<T> other)
        {
            if (other == null)
            {
                throw new ArgumentNullException(nameof(other));
            }

            // a set is equal to itself
            if (other == this)
            {
                return true;
            }

            HashSet<T> otherAsSet = other as HashSet<T>;
            // faster if other is a hashset and we're using same equality comparer
            if (otherAsSet != null && AreEqualityComparersEqual(this, otherAsSet))
            {
                // attempt to return early: since both contain unique elements, if they have 
                // different counts, then they can't be equal
                if (_count != otherAsSet.Count)
                {
                    return false;
                }

                // already confirmed that the sets have the same number of distinct elements, so if
                // one is a superset of the other then they must be equal
                return ContainsAllElements(otherAsSet);
            }
            else
            {
                ICollection<T> otherAsCollection = other as ICollection<T>;
                if (otherAsCollection != null)
                {
                    // if this count is 0 but other contains at least one element, they can't be equal
                    if (_count == 0 && otherAsCollection.Count > 0)
                    {
                        return false;
                    }
                }
                ElementCount result = CheckUniqueAndUnfoundElements(other, true);
                return (result.uniqueCount == _count && result.unfoundCount == 0);
            }
        }

[gokhanabatay]

Below code of HashSet returns false, in NH QueryKey line 130 of CollectionsHelper class.
But I did not understand of GetHashCode login at FilterKey class so different instance of Filter has different HashCodes so it did not call equals method because hashcodes are not equal.

How can I fix it, please let me.

    // Checks if this hashset contains the item
    public bool Contains(T item)
    {
        if (_buckets != null)
        {
            int collisionCount = 0;
            int hashCode = InternalGetHashCode(item);
            Slot[] slots = _slots;
            // see note at "HashSet" level describing why "- 1" appears in for loop
            for (int i = _buckets[hashCode % _buckets.Length] - 1; i >= 0; i = slots[i].next)
            {
                if (slots[i].hashCode == hashCode && _comparer.Equals(slots[i].value, item))
                {
                    return true;
                }

                if (collisionCount >= slots.Length)
                {
                    // The chain of entries forms a loop, which means a concurrent update has happened.
                    throw new InvalidOperationException(SR.InvalidOperation_ConcurrentOperationsNotSupported);
                }
                collisionCount++;
            }
        }
        // either _buckets is null or wasn't found
        return false;
    }

[gokhanabatay]

@hazzik please let me an option to fix this issue.
@fredericDelaporte as you say that the issue is GetHashCode function. Let me tell you.
I debug all .net core HashSet among other class there are lots of codes...

Below code CoreDistributedCache line 232 at Deserialize calls our "FilterKey GetHashCode" method and "_filterParameters" is empty when it called. And wrong HashCode is calculated and stored at HashSet local variable.
This is the issue that your test does not effected and succeeded but in life scenario it fails, because in your test you do not deserialize Filters you just initialize them.

var serializer = new BinaryFormatter(); using (var stream = new MemoryStream(cachedData)) { var entry = serializer.Deserialize(stream) as Tuple<object, object>; return Equals(entry?.Item1, key) ? entry.Item2 : null; }

FilterKey
public override int GetHashCode() { var result = 13; result = 37 * result + _filterName.GetHashCode(); result = 37 * result + CollectionHelper.GetHashCode(_filterParameters, NamedParameterComparer.Instance); return result; }

As a Result I think best solution is SequenceEquals because it does not care HashCode it just cares Equals method and it works like charm as i said :) I Ordered filters for your tip.

if (!CollectionHelper.SequenceEquals(_filters?.OrderBy(x=>x.ToString()) , that._filters?.OrderBy(x=>x.ToString()))) { return false; }

[fredericDelaporte]

For helping pushing the subject forward, the only way is still to start by adding a test case.

This is the issue that your test does not effected and succeeded but in life scenario it fails, because in your test you do not deserialize Filters you just initialize them.

Then add a test which does such an operation before comparing them. There are already many tests doing some serialization/deserialization in the test project, just look for Serialize. Then you could add your test into NHibernate.Test.CacheTest.FilterKeyFixture.

[fredericDelaporte]

That is still a workaround, and performance wise, a not efficient one.

The root cause should be fixed instead.

[fredericDelaporte]

I am looking into this. Re-reading your statements, it seems it is an issue with QueryKey serialization and its hashcode deserialization. So the test case should be added into QueryKeyFixture instead. I expect it to be hard to reproduce, because I think the trouble is not the hashcode being recomputed with an empty hashset, but being shared among processes through the serialization/deserialization into the core distributed cache, while it is not guaranteed to be stable between processes.

[fredericDelaporte]

Replaced by #1891