1

Behavior:

I have been recently solving problem, which took me whole day to solve. After a lot of debugging, I have found it is related to Changing values in HashSet

One of colleagues changed field contained in hashcode and the problem appeared after a few months in one specific case. This is really complex application, team consists of 100 developers and code-review does not reveal everything.

Question:

Does exists any code analyzer (ASM manipulator, Aspect, whatever) with ability to detect manipulation with hashCode() for elements which are part of HashSet?

Our application has nearly 100% coverage in Unit tests, so it could be any kind of Aspect, registered for test context only.

Or:

Is there any Collection implementation, which is taking the best from HashSet (at least lookup complexity O(1)) and:

  • Throws exception on hashcode change - Just curiosity, I really have no idea how could Collection detect it.
  • Or at least returns false in case removeIf doesnt remove element? (only fallback solution)

Or:

How to prevent this situation correctly?

Adding simple test to show, what I am trying to accomplish. In real world, it is not Date class and and between setX() and removeIf are tens of classes and thousands of other rows:

@Test
public void collectionTest(){
    testCollectionOfType(new HashSet<>()); // add any implementation of Collection with complexity O(1)
}

// Looking for something fail-fast that throws Exception (Not assertion error) when hashcode changed
// Or Collection with complexity O(1) which pass following test
public void testCollectionOfType(Collection<Date> test){
    test.add(new Date());

    test.iterator().next().setTime(123123123); // In ideal world, this would be detected by any kind of analyzer or throw exception

    TestCase.assertFalse(test.removeIf(s -> true));  // Fail: returns true, because Predicate matched. Does not remove enything, because Iterator#remove not found record in hash table.
    TestCase.assertFalse(test.removeAll(test)); // Fail: Same situation, returns true and did not removed anything :-(

    Iterator<Date> it = test.iterator();
    while(it.hasNext()){
        it.next();
        it.remove();
    }

    TestCase.assertEquals(0,test.size());   // Fail: Set is still not empty
}
Bedla
  • 4,789
  • 2
  • 13
  • 27
  • 1
    This is why immutable objects are useful. – Kayaman Dec 04 '17 at 10:22
  • General approach would be to avoid using mutable types as keys. – Oliver Charlesworth Dec 04 '17 at 10:22
  • @OliverCharlesworth of course in a `HashSet` they're not really keys, except in the implementation. – Kayaman Dec 04 '17 at 10:24
  • 2
    Ah, true. Well, "avoid using mutable types in general", then :) – Oliver Charlesworth Dec 04 '17 at 10:25
  • You are completely right, This is best prevention. And what now? Is there any way how to get out from this without rewriting all DTO objects? – Bedla Dec 04 '17 at 10:33
  • I can't imagine how a fail-fast approach might work. I guess you could periodically run sanity checks on your set(s) - keep a record of the mapping from object identity to hashcode, and if an entry ever changes, you know something bad happened. – Oliver Charlesworth Dec 04 '17 at 12:28
  • 1
    we "sort-of" have this - some third party objects that constantly change and might break our sets/maps too. what we did is write unit tests on the fields that generate the hashCode. Exactly. decompile them - look at the fields that make up the hashCode and write unit tests for that code. So say you have `class User {int age, String name}` and hashCode is made from those two `age` and `name` - we have 10-25 different methods that generate hashCode/equals and test them. If a future release adds some fields - these tests will fail... – Eugene Dec 04 '17 at 20:28

0 Answers0