Issue
Context:
There is an Infinispan (13.0.10) cache that uses a custom data type Quad<String, Long, Type, ValidityPeriod> as key. As far as I can tell all hashCode() and equals(Object o)-methods are either auto-generated or implemented correctly.
Also the cache is used in Local Mode.
Question:
When I try to call cache.get(Object key) with a Quad that I know the cache contains, I get null. cache.containsKey(Object key) also returns false.
How is this possible?
Clarification:
I said that I know the cache contains the Quad key because I have done the following using the Eclipse IDE debugger:
cache.entrySet().stream().filter(e -> e.getKey().hashCode == Quad.of(a, b, c, d).hashCode()).collect(toList());
// this returns a non-empty list with my expected return
cache.entrySet().stream().filter(e -> e.getKey().equals(Quad.of(a, b, c, d))).collect(toList());
// this returns a non-empty list with my expected return
Per Radim Vansa's suggestion I have also tried the following:
cache.entrySet().stream().collect(Collectors.toMap(Entry::getKey, Entry::getValue, (o1,o2) -> o1, ConcurrentHashMap::new)).get(Quad.of(a, b, c, d));
// this returns my expected return
Further Context (if needed): I am working on an older project where I am supposed to update from Infinispan Version 10 to 13. I have successfully done so and also integrated ProtoStream API instead of using the old JBossMarshaller. This version change is important because the retrieval stopped working after the update.
There are multiple Caches in use, some are indexed using custom data types such as Pair<K, V>, Triple<L, M, R> and Quad<A, B, C, D> which are all generic. I have written some ProtoAdpaters for all of them and they work just fine.
I now have come across a problem, where there is an AdvancedCache that uses a Quad like this as key:
Quad<String, Long, Type, ValidityPeriod>
Quad overrides equals(Object o) and hashCode as follows:
public class Quad<A, B, C, D> {
// other code omitted for brevity
public boolean equals(final Object obj) {
if (obj == this) {
return true;
}
if (obj instanceof Quad<?, ?, ?, ?>) {
final Quad<?, ?, ?, ?> other = (Quad<?, ?, ?, ?>) obj;
return Objects.equals(getFirst(), other.getFirst())
&& Objects.equals(getSecond(), other.getSecond())
&& Objects.equals(getThird(), other.getThird())
&& Objects.equals(getFourth(), other.getFourth());
}
return false;
}
public int hashCode() {
return Objects.hashCode(getFirst())
^ Objects.hashCode(getSecond())
^ Objects.hashCode(getThird())
^ Objects.hashCode(getFourth());
}
}
For reference Type is structured along the lines of:
public class Type implements Serializable {
private int fieldA;
private int fieldB;
private String fieldC;
// hashCode and equals are auto-generated
// constructor, getters and setters omitted for brevity
}
ValidityPeriod is something like this:
public class ValidityPeriod implements Serializable {
private LocalDate validFrom;
private LocalDate invalidFrom;
// hashCode and equals are auto-generated
// constructor, getters and setters omitted for brevity
}
The marshaller for LocalDate uses the following adapter:
@ProtoAdapter(LocalDate.class)
public class LocalDateAdapter {
@ProtoFactory
LocalDate create(int year, short month, short day) {
return LocalDate.of(year, month, month);
}
@ProtoField(number = 1, required = true)
int getYear(LocalDate localDate) {
return localDate.getYear();
}
@ProtoField(number = 2, required = true)
short getMonth(LocalDate localDate) {
return (short) localDate.getMonth().getValue();
}
@ProtoField(number = 3, required = true)
short getDay(LocalDate localDate) {
return (short) localDate.getDayOfMonth();
}
}
I have tried to use the debugger to understand the internal workings of Infinispan but I don't seem to be able to pin down a concrete line producing this error.
As far as I know it has something to do with CacheImpl.get(Object, long, InvocationContext).
Update:
Ok, per Pruivo's suggestion I have tried to create a reprex. The strange thing however is that I have tried to copy every process that happens before the object is retrieved from the cache, but in the reprex I created it works.
Funny thing however is the following I tried:
I created two methods in ValidityPeriod that do the following (almost like Infinispan Transformers):
public String toFormString() {
return String.format("%s§%s", validFrom, invalidFrom);
}
public static ValidityPeriod fromFormString(String form) {
String[] split = form.split("§");
return from(LocalDate.parse(split[0]),LocalDate.parse(split[1]));
}
Then I've changed the Quad to Quad<String, Long, Type, String> while constructing the key for the cache with strings from these methods instead of the ValidityPeriod itself. Strangely enough, this fixes my original problem.
However, as this is a dirty fix, I am not content with keeping this solution permanent. There has to be something wrong with ValidityPeriod is my guess.
I still am confused as to why the cache returns something different than it contains, so I will still leave my original question open.
Solution
After a careful deduction following Pruivo's suggestions, I could pin down a cause for this bug. It is the ProtoAdapter that is written for LocalDate in which an incorrect LocalDate-object is created upon deserialization.
Concrete fix: The concrete fix for this case is the following:
@ProtoAdapter(LocalDate.class)
public class LocalDateAdapter {
@ProtoFactory
LocalDate create(int year, short month, short day) {
return LocalDate.of(year, month, day); // <= here was the error
}
@ProtoField(number = 1, required = true)
int getYear(LocalDate localDate) {
return localDate.getYear();
}
@ProtoField(number = 2, required = true)
short getMonth(LocalDate localDate) {
return (short) localDate.getMonth().getValue();
}
@ProtoField(number = 3, required = true)
short getDay(LocalDate localDate) {
return (short) localDate.getDayOfMonth();
}
}
Explanation:
As Pruivo stated, Infinispan uses hashCode() to determine a key's owner/segment. In the background Infinispan uses a ConcurrentHashMap for storing keys and values. This map contains serialized objects if the cache is configured for that.
Now to the problem. The ConcurrentHashMap contains the following method that is called to retrieve an object:
public V get(Object key) {
Node<K,V>[] tab; Node<K,V> e, p; int n, eh; K ek;
int h = spread(key.hashCode());
if ((tab = table) != null && (n = tab.length) > 0 &&
(e = tabAt(tab, (n - 1) & h)) != null) { // <= line 5
// this code here is not reached
if ((eh = e.hash) == h) {
if ((ek = e.key) == key || (ek != null && key.equals(ek)))
return e.val;
}
else if (eh < 0)
return (p = e.find(h, key)) != null ? p.val : null;
while ((e = e.next) != null) {
if (e.hash == h &&
((ek = e.key) == key || (ek != null && key.equals(ek))))
return e.val;
}
}
}
return null;
}
As you can see in line 5, there is a check for a method called tabAt(). This checks if the object reference at that adress of the map is null. If so then no further actions are taken and null will be returned. This includes that no check for hashCode() or equals() are done under this condition. This you can see in the lines that follow tabAt(). So this situation explains why cache.entrySet().stream()... could retrieve an object an get() couldn't.
Now my educated guess why the object's reference isn't found: My quess is that the original key is that of the correctly serialized object but after deserialization this key becomes corrupt as it is no longer the correct one. As modification of a key in a Map can produce unexpected behaviour, this is my theory. (Source for unexpected behaviour of Map when key is modified: https://stackoverflow.com/a/21601013/14945497)
Suggestion/Remark: A final remark I would like to make is that this error can be quite inconspicuous and lead to problems down the line because wrong objects are created upon deserialization. This leads to a lot of detective work when an error does occur. So, double-triple-check your definitions for all marshallers is my suggestion.
Answered By - Thomas
Answer Checked By - Timothy Miller (JavaFixing Admin)
