JetBrains / lincheck

Framework for testing concurrent data structures
Mozilla Public License 2.0
572 stars 33 forks source link

Hash code stub transformer implementation is incorrect #131

Closed kkarnauk closed 7 months ago

kkarnauk commented 1 year ago

Version: 2.16 (and the previous ones, too)

Issue

As you can see in the implementation of HashCodeStubTransformer, in order to understand whether it should change a call of hashCode, it checkes owner. But it doesn't take into account that owner means the actual callee on the stack (not the concrete class which method will be called).

So, there are following problems with that:

  1. If we cast an object to Any/Object with overriden hashCode and call it, then 0 is returned. For example:

    val a: Any = 1
    val b = 1
    println(a.hashCode()) // prints 0
    println(b.hashCode()) // prints 1

    I suppose, it is a huge hole. First of all, it becomes inconistent and the equals-hashCode invariant doesn't work. Also, in most collections all objects are casted to Any/Object, so all hashCode invocations return 0 and collections start working slow.

  2. If we have a class that doesn't override hashCode (and so do its parents) and owner != Object, then no transformation is done. It still calls the native implementation (which usually returns the memory address) and we have the non-determinism.

How to fix

I have the following proposal. There are two fixes (one for each problem):

  1. We check that owner is Any/Object and if it's true, we check ::class of the value on the stack. If it is actually Object/Any, then we need to do the transformation.
  2. We need another class transformer. It should check if there is a class which inherits Object directly and doesn't override the default hashCode(), it should add an implementation of hashCode which returns 0.
ndkoval commented 1 year ago

To solve the issue, we need to track down the classes that override hashCode. One unreliable but simple solution would be to check whether obj.hashCode() == System.identityHashCode(obj) to detect that hashCode() returns the identity hash code. Thanks to @kkarnauk for the suggestion. While this simple solution is imperfect, it is super-easy to implement and should work fine in practice.

ndkoval commented 1 year ago

The main question is "what System.identityHashCode(obj) should return?". I suggest generating a pseudo-random number and associating it with the object for further calls.

ndkoval commented 8 months ago

Should be resolved in #249