Why do 2 delegate instances return the same hashcode?

Question

Take the following:

  var x =  new Action(() => { Console.Write("") ; });
  var y = new Action(() => { });
  var a = x.GetHashCode();
  var b = y.GetHashCode();
  Console.WriteLine(a == b);
  Console.WriteLine(x == y);

This will print:

True
False

Why is the hashcode the same?

It is kinda surprising, and will make using delegates in a Dictionary as slow as a List (aka O(n) for lookups).

Update:

The question is why. IOW who made such a (silly) decision?

A better hashcode implementation would have been:

return Method ^ Target == null ? 0 : Target.GetHashcode();
// where Method is IntPtr

Did you try to put a some code into second delegate too , and only after check Hash code ? — Tigran, Jul 08 '11 at 12:05
Don't know exactly WHY it happens, but just as an idea, you may implement your own Hash mechanism, or wrap those actions in a class and override its GetHashCode to fit your needs. — Can Poyrazoğlu, Jul 08 '11 at 12:06
Just for reference the contract for `Delegate.Equals` is: "Determines whether the specified object and the current delegate are of the same type and share the same targets, methods, and invocation list." — CodesInChaos, Jul 08 '11 at 12:09
@leppie although I must confess that I have never had a scenario where I wanted to use a delegate as a *key* in a dictionary. As the value, *for sure* - just not the key. — Marc Gravell, Jul 08 '11 at 12:30
Your "better implementation" has the same mistake as the one I originally made: You're calling the overriden `Target.GetHashCode` instead of using referential equality. — CodesInChaos, Jul 08 '11 at 12:46
I'm currently building a simple task scheduler, that takes `Action` as input. Trying to figure out a way to UNIQUELY identify actions. Haven't found one yet :( — Alex from Jitbit, Nov 09 '17 at 21:16

Aliostad · Accepted Answer · 2011-07-08T12:24:01.010

11

Easy! Since here is the implementation of the GetHashCode ~~(sitting on the base class Delegate):~~

public override int GetHashCode()
{
    return base.GetType().GetHashCode();
}

(sitting on the base class MulticastDelegate which will call above):

public sealed override int GetHashCode()
{
    if (this.IsUnmanagedFunctionPtr())
    {
        return ValueType.GetHashCodeOfPtr(base._methodPtr);
    }
    object[] objArray = this._invocationList as object[];
    if (objArray == null)
    {
        return base.GetHashCode();
    }
    int num = 0;
    for (int i = 0; i < ((int) this._invocationCount); i++)
    {
        num = (num * 0x21) + objArray[i].GetHashCode();
    }
    return num;
}

Using tools such as Reflector, we can see the code and it seems like the default implementation is as strange as we see above.

The type value here will be Action. Hence the result above is correct.

UPDATE

edited Jul 08 '11 at 12:24

answered Jul 08 '11 at 12:15

Aliostad

80,612
21
160
208

5

To be honest `return 42;` would be *about* as useful as the `Delegate` implementation (although `MulticastDelegate` looks better) – Marc Gravell Jul 08 '11 at 12:21
However, `GetHashCode` is overridden in turn on `MulticastDelegate`, which adds up the hash codes of all the subscribed delegates (if it's actually a multicast delegate). The plot thickens... – thecoop Jul 08 '11 at 12:23
1

@Marc: `Object.GetType()` does a dynamic lookup of the actual object type at runtime, so it isn't the same as `typeof(object).GetHashCode()`. It actually returns different values for each delegate type (but the same value for all instances of that type) – thecoop Jul 08 '11 at 12:25
@Marc: I just scrubbed my comment; The `MulticastDelegate` implementation is just as broken because the combined hashcode will be generated by calling up to the dodgy base implementation for each target. You'll get different hashcodes if you have different *numbers* of targets, but if you have the same number of targets then you're going to get the same hashcode! – LukeH Jul 08 '11 at 12:26
1

@thecoop: And how often to you subscribe multiple methods to a delegate in a non-event situation? I can probably count the number of times on half a hand :) – leppie Jul 08 '11 at 12:26
@thecoop - yeah, scrubbed that – Marc Gravell Jul 08 '11 at 12:32

CodesInChaos · Answer 2 · 2011-07-08T12:45:11.923

3

My first attempt of a better implementation:

public class DelegateEqualityComparer:IEqualityComparer<Delegate>
{
    public bool Equals(Delegate del1,Delegate del2)
    {
        return (del1 != null) && del1.Equals(del2);
    }

    public int GetHashCode(Delegate obj)
    {
            if(obj==null)
                return 0;
            int result = obj.Method.GetHashCode() ^ obj.GetType().GetHashCode();
            if(obj.Target != null)
                result ^= RuntimeHelpers.GetHashCode(obj);
            return result;
    }
}

The quality of this should be good for single cast delegates, but not so much for multicast delegates (If I recall correctly Target/Method return the values of the last element delegate).

But I'm not really sure if it fulfills the contract in all corner cases.

Hmm it looks like quality requires referential equality of the targets.

edited Jul 08 '11 at 12:45

answered Jul 08 '11 at 12:26

CodesInChaos

106,488
23
218
262

@adrianm: Your comment does not make sense. – leppie Jul 08 '11 at 12:36
Hmm interesting thought @adrianm. I think the specification of equals might mean referential equality instead of `.Equals()` – CodesInChaos Jul 08 '11 at 12:39
1

Modified it to use referential equality of targets. – CodesInChaos Jul 08 '11 at 12:45
@leppie: If something within the target changes, the delegate's hashcode will also change, i.e. you break the contract of dictionary keys. I can't pinpoint exactly why but I think the strange hashcode implementation has something to do with the equals definition and immutable invocation lists. – adrianm Jul 08 '11 at 12:47
@adrianm: I understand what you mean, but one should never use keys like that! – leppie Jul 08 '11 at 13:06
@CodesInChaos Could you do this -> public int GetHashCode(Delegate? obj) { if (obj == null) return 0; else { long asdf = Marshal.GetFunctionPointerForDelegate(obj).ToInt64(); return (int)(asdf % 920574309); } } – Treker Sep 21 '22 at 07:06

score 1 · Answer 3 · edited May 23 '17 at 10:29

1

This smells like some of the cases mentioned in this thread, maybe it will give you some pointers on this behaviour. else, you could log it there :-)

What's the strangest corner case you've seen in C# or .NET?

Rgds GJ

edited May 23 '17 at 10:29

Community

1
1

answered Jul 08 '11 at 12:06

gjvdkamp

9,929
3
38
46

2

which post, specifically? it isn't related to the one in the question, if that is what you mean – Marc Gravell Jul 08 '11 at 12:12

score 0 · Answer 4 · answered Jul 08 '11 at 12:09

0

From MSDN :

The default implementation of GetHashCode does not guarantee uniqueness or consistency; therefore, it must not be used as a unique object identifier for hashing purposes. Derived classes must override GetHashCode with an implementation that returns a unique hash code. For best results, the hash code must be based on the value of an instance field or property, instead of a static field or property.

So if you have not overwritten the GetHashCode method, it may return the same. I suspect this is because it generates it from the definition, not the instance.

answered Jul 08 '11 at 12:09

Schroedingers Cat

3,099
1
15
33

5

That the contract allows this is obvious. The question is why the implementation is this bad. – CodesInChaos Jul 08 '11 at 12:11
My point was that MS evade this by saying you need to override the default implementation to get unique values. It means that the default implementation is useless. As Aliostad points out above, and I tried to say, the default is based on the type, not the instance. It is probably because MS hates us. – Schroedingers Cat Jul 08 '11 at 12:21
Now if only we could override the hashcode in C#. I know it is possible in the CLR (via IL) to construct custom (and very slow) delegates. – leppie Jul 08 '11 at 12:21
3

@leppie: If you're concerned about performance in hashtables etc then you could just pass an `IEqualityComparer` that "does the right thing" into your `Dictionary`/`HashSet` etc constructors, or knock-up custom `DelegateDictionary`/`DelegateHashSet` that do it automatically. – LukeH Jul 08 '11 at 12:30
@leppie: Admittedly it's not ideal, and something you really shouldn't need to do in the first place, but it's probably an adequate workaround. – LukeH Jul 08 '11 at 12:31
@LukeH: I agree, and will do that if ever needed :) – leppie Jul 08 '11 at 12:32
@LukeH to do that we first need to figure out what the right thing is. And that's not that easy. So it'd be really nice if MS supplied a good default implementation. – CodesInChaos Jul 08 '11 at 12:47

Why do 2 delegate instances return the same hashcode?

4 Answers4

UPDATE

Linked