ebean icon indicating copy to clipboard operation
ebean copied to clipboard

Hashcode generation throws IndexOutOfBoundsException

Open sybren-gjaltema-2020 opened this issue 1 year ago • 3 comments

Expected behavior

We have an Entity for which we want to generate the hashcode. The hashcode is generated over a List which contains other beans.

Actual behavior

An exception is thrown. The exception is the following:

 java.lang.IndexOutOfBoundsException: The size must be at least 1 
     at io.ebeaninternal.server.deploy.id.IdBinderSimple.getIdInValueExpr(IdBinderSimple.java:135) 
     at io.ebeaninternal.server.deploy.BeanDescriptor.parentIdInExpr(BeanDescriptor.java:1634) 
     at io.ebeaninternal.server.deploy.BeanPropertyAssocManySqlHelp.addWhereParentIdIn(BeanPropertyAssocManySqlHelp.java:121) 
     at io.ebeaninternal.server.deploy.BeanPropertyAssocMany.addWhereParentIdIn(BeanPropertyAssocMany.java:342) 
     at io.ebeaninternal.api.LoadManyRequest.createQuery(LoadManyRequest.java:90) 
     at io.ebeaninternal.server.core.DefaultBeanLoader.loadMany(DefaultBeanLoader.java:38) 
     at io.ebeaninternal.server.core.DefaultServer.loadMany(DefaultServer.java:474) 
     at io.ebeaninternal.server.loadcontext.DLoadManyContext$LoadBuffer.loadMany(DLoadManyContext.java:222) 
     at io.ebean.common.AbstractBeanCollection.lazyLoadCollection(AbstractBeanCollection.java:90) 
     at io.ebean.common.BeanList.init(BeanList.java:141) 
     at io.ebean.common.BeanList.hashCode(BeanList.java:217) 
     at java.base/java.util.Arrays.hashCode(Arrays.java:4706) 
     at java.base/java.util.Objects.hash(Objects.java:147) 
     at mypackage.MyClass.hashCode(MyClass.java) 

The bean looks a little something like this:

package mypackage;

import javax.persistence.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

@DiscriminatorValue("MyClass")
@Entity
@Inheritance
public abstract class MyClass {

    @OrderBy("id")
    @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true)
    @JoinTable(name = "other_bean", inverseJoinColumns = @JoinColumn(name = "other_bean_id"))
    private final List<OtherBean> otherBeans = new ArrayList<>();

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof MyClass myClass)) return false;
        return Objects.equals(otherBeans, myClass.otherBeans);
    }

    @Override
    public int hashCode() {
        return Objects.hash(otherBeans);
    }
}

We are using ebean 13.9.2. If any more information is needed to track down the problem, I'd be glad to provide it to you.

sybren-gjaltema-2020 avatar Sep 13 '22 13:09 sybren-gjaltema-2020

The code overrides hashCode() but not equals() ?
Can you edit that and include the equals implementation?
Could you also describe what we are trying to achieve here with hashCode in terms of what is goal / reason for doing this?

The reason I say that is because hashCode() has specific semantics and the implementation here looks like it breaks those semantics. For example, hashCode() and equals() are used internally by Set [and Map keys] and the hashCode value determines the internal bucket that an entry goes in. The implication is that the hashCode() value should not change, if it changes then we can get situations like Set::exists() does not work, an instance can be held in a set more than once etc. To me it looks like the semantics of hashCode() are not being honored here.

rbygrave avatar Sep 13 '22 20:09 rbygrave

Hi! Thanks for your swift response. Sorry, I only added the part of the code I thought was relevant. The equals() check works similarly to hashcode(). I have added the equals snippet to the ticket. I understand what hashcode() is used for, but I fail to see the relevance to the thrown exception. Even if my implementation of hashcode() was nondeterministic (which it shouldn't be), I still wouldn't expect Ebean to throw an IOOBE.

sybren-gjaltema-2020 avatar Sep 14 '22 07:09 sybren-gjaltema-2020

Can you produce a failing test case so that someone can run it?

rbygrave avatar Sep 14 '22 20:09 rbygrave

I have been unable to reproduce this in a test. It still happens consistently, even after upgrading to ebean 13.9.3. Having jumped into debugging mode, I noticed that the method parentIdList in LoadManyRequests returns an empty list, which then gets passed down all the way to IdBinderSimple, where our exception is thrown (because the list is empty). As I understood this, this list appears to be the parent classes of the data to load, and should therefore never be empty (?). Any suggestions (or any suggestions to possibly catch this in a test)?

sybren-gjaltema-2020 avatar Sep 27 '22 12:09 sybren-gjaltema-2020

An empty parentIdList implies the LoadManyRequests batch is empty. That implies the batch was always empty or that the batch was previously lazy loaded (or some sort of recursion where lazy loading is invoking other lazy loading which might well be possible here).

In terms of stack trace we can't see what happens before the MyClass.hashCode()[as that part of the stack trace has been truncated]. For example, is there a lot more stack trace and that shows some recursion going on? I'd be interested in what actually calls the MyClass.hashCode() myself because it's still not clear what we are trying to achieve here (why the implementation of hashCode/equals is like this / non-deterministic. Specifically, to me it looks like the hashCode/equals should instead be specific methods used for a specific purpose - some sort of comparator? That is, hashCode/equals is used implicitly by Set and Map implementations etc and as such it looks like this implementation presents risks.

In terms of reproducing it you might present some of the logic that occurs to build the MyClass instance before hashCode is called (it looks like it is queried from the database, we can't see the OtherBean model yet etc).

rbygrave avatar Sep 27 '22 19:09 rbygrave

Hi again,

I attempted to remove all custom implementations of hashCode() and equals(), and the stacktrace does change - it no longer breaks on the exact same line, but still breaks in a similar fashion. The stracktrace is the following:

java.lang.IndexOutOfBoundsException: The size must be at least 1
	at io.ebeaninternal.server.deploy.id.IdBinderSimple.getIdInValueExpr(IdBinderSimple.java:135) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.deploy.BeanDescriptor.parentIdInExpr(BeanDescriptor.java:1634) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.deploy.BeanPropertyAssocManySqlHelp.addWhereParentIdIn(BeanPropertyAssocManySqlHelp.java:121) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.deploy.BeanPropertyAssocMany.addWhereParentIdIn(BeanPropertyAssocMany.java:342) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.api.LoadManyRequest.createQuery(LoadManyRequest.java:90) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.core.DefaultBeanLoader.loadMany(DefaultBeanLoader.java:38) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.core.DefaultServer.loadMany(DefaultServer.java:472) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebeaninternal.server.loadcontext.DLoadManyContext$LoadBuffer.loadMany(DLoadManyContext.java:222) ~[ebean-core-13.9.3.jar:13.9.3]
	at io.ebean.common.AbstractBeanCollection.lazyLoadCollection(AbstractBeanCollection.java:90) ~[ebean-api-13.9.3.jar:13.9.3]
	at io.ebean.common.BeanList.init(BeanList.java:141) ~[ebean-api-13.9.3.jar:13.9.3]
	at io.ebean.common.BeanList.iterator(BeanList.java:327) ~[ebean-api-13.9.3.jar:13.9.3]
	at java.util.Spliterators$IteratorSpliterator.estimateSize(Spliterators.java:1821) ~[?:?]
	at java.util.Spliterator.getExactSizeIfKnown(Spliterator.java:408) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:483) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?]
	at mypackage.MyClass.getHandle(MyClass.java:141) ~[classes/:?]

where getHandle() is implemented like this:

    public synchronized List<String> getHandle() {
        ArrayList<String> handle = new ArrayList<>();
        otherBeans.stream().map(OtherBean::getValue).forEach(handle::add); <-- Exception thrown here
        // some other stuff
        return handle;
    }

In regards to recursion, in this case there is no recursion. The stacktrace is truncated, but any of the other classes in the stacktrace are not @Entity's. getHandle() is called on a Class that extends MyClass, but it doesn't override the getHandle function (or any function for that matter).

I'll attempt writing a test case again next week if time is allotted to me to continue attempting to fix this issue. Thanks for your help so far!

sybren-gjaltema-2020 avatar Oct 07 '22 12:10 sybren-gjaltema-2020

attempt writing a test case again next week if time is allotted to me to continue attempting to fix this issue

Yup cool.

Note that this is about how otherBeans is created - how it gets into the state that it's in when this getHandle() is called. It very much looks like otherBeans.size() could produce a similar error as the otherBeans.stream(). ... as it looks like it is lazy loading the collection when it is in some invalid state where the parent is unknown/not set.

Its a question of how otherBeans is created (and you have not shared that code with us yet).

rbygrave avatar Oct 10 '22 00:10 rbygrave