Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hibernate broken in 3.5.3 #3586

Closed
JohannesBeranek opened this issue Nov 21, 2024 · 3 comments
Closed

Hibernate broken in 3.5.3 #3586

JohannesBeranek opened this issue Nov 21, 2024 · 3 comments

Comments

@JohannesBeranek
Copy link
Contributor

JohannesBeranek commented Nov 21, 2024

Hibernate doesn't seem to work using TransactionalRequest in 3.5.3 with enabledDefault = false and @Transactional annotation:

TransactionalRequest apply has this code:

...
try (var session = sessionProvider.create(ctx, sessionFactory)) {

io.jooby.internal.hibernate.RequestSessionFactory.StatefulSessionFactory provides this create method, which is called:

    public SharedSessionContract create(Context ctx, SessionFactory sessionFactory) {
      var sessionProvider = ctx.require(sessionProviderKey);
      var session = sessionProvider.newSession(sessionFactory.withOptions());
      return ManagedSessionContext.bind(session);
    }

org.hibernate.context.internal.ManagedSessionContext bind is implemented as:

    public static Session bind(Session session) {
        return (Session)sessionMap(true).put(session.getSessionFactory(), session);
    }

sessionMap will return a HashMap, and put in HashMap calls putVal (though that may be JDK dependent - latest temurin JDK 21 in my case) which will return the PREVIOUS value of that key, or null, if there was no previous value. The spec also seems to support this behavior, so basically all JDKs should behave this way.

So the code will result in an NPE, once TransactionalRequest tries to call anything on session - like it does in the apply method (the same I mentioned at the top):

            trx = session.getTransaction();

I think it would be correct to fix ManagedSessionContext to return the new session, and not the previous value, OR in StatefulSessionFactory return the session, instead of the previous session, or ...

Note that this has probably been broken for some time already, as I couldn't find a recent commit that changed the code and we upgraded from 3.1.1 to 3.5.3 and I didn't want to go through hundreds of commits, as bisecting didn't yield anything meaningful

@JohannesBeranek JohannesBeranek changed the title Hibernate broken Hibernate broken in 3.5.3 Nov 21, 2024
@JohannesBeranek
Copy link
Contributor Author

Should probably be broken up into two lines, similar to how it is in StatelessSessionFactory, so I would assume this actually broke in the last release

@jknack
Copy link
Member

jknack commented Nov 21, 2024

Nov 21, 2024

Right. I broke on latest (introducing statelesssession)

jknack added a commit that referenced this issue Nov 21, 2024
#3586 Fix RequestSessionFactory.StatefulSessionFactory create returning null session
@jknack
Copy link
Member

jknack commented Nov 21, 2024

Will be on central in 10 minutes or less.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants