Handle/prevent event loop switching in Hibernate Reactive

[yrodiere]

After #2690, it appears that under some conditions, execution might suddenly move to a different thread / event loop. This has only been observed when opening connections lazily.

This breaks a core assumption that we verify here:

hibernate-reactive/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/impl/ReactiveSessionImpl.java

Lines 201 to 203 in 2fffcce

	private void threadCheck() {
	InternalStateAssertions.assertCurrentThreadMatches( associatedWorkThread );
	}

... and results in failures as described there:

#2690 (comment)

We have two options.

Find out why we're switching event loops

If we consider that event loop switching is unexpected / unacceptable, then should work on understanding why we're switching event loops exactly.

@tsegismont may be able to help here but we will need to provide more information.

I personally suspect that connection opening (sometimes?) leads to event loop switching, but I cannot back that claim at the moment, and @DavideD mentioned that the problem happens on session closing, which would prove me wrong.

Revise our checks

It's been mentioned in the past that our checks are too strict.

I believe what we actually want to check is that:

1. We're not using a session with a different context active than the one used to create the session.
2. We're never using the same session from multiple event loops at the same time. (Perhaps not the initial intent, but we're effectively checking it and it's a nice bonus).

Both should generally be the case, but then users could mistakenly end up using a Session instance on some worker thread, or passing it around to different reactive chains in exotic ways. And it would result in hard-to-debug issues.

#2494 will make the second check pretty much irrelevant by making sure we never execute two session operations at the same time. At least for the reactive ones, and I think it's fair to leave out other operations and assume any concurrent calls on those are user errors.

For the context, we could perhaps replace the current check based on the thread, with one based on the context? For instance:

1. Store the current context on session creation, and check before every operation that it's active (ContextInternal#isRunningOnContext()).
2. Store the session token (session.getSessionToken()) in the context on session creation, and check before every operation that the token is present in the context . Note we'd have to use a Set, because multiple sessions (e.g. from multiple PUs) can be active at the same time.

The second solution seems more flexible because it would allow duplicating the context during the session's lifetime, which is something we're planning to do to implement quarkusio/quarkus#47698.

[DavideD]

A little clarification about

but then users could mistakenly end up using a Session instance on some worker thread

We check if the session is used in an event loop thread, so a users cannot use it in a worker thread (unless they disable the checks)

[yrodiere]

Right, I had forgotten about that.

Well, that bit is still valid at least:

or passing it around to different reactive chains in exotic ways

[DavideD]

I personally suspect that connection opening (sometimes?) leads to event loop switching, but I cannot back that claim at the moment, and @DavideD mentioned that the problem happens on session closing, which would prove me wrong.

I think you are right about the problem being during connection opening. Yesterday I didn't realize that the error during close wasn't the only error thrown (sorry). I'm double checking now

[gavinking]

We check if the session is used in an event loop thread, so a users cannot use it in a worker thread (unless they disable the checks)

Wait, what, why?

Is there really something intrinsically wrong with using a session on a worker thread, if it started out on that thread?

Why exactly did we do that?

[DavideD]

I think it was a precaution to prevent users to have the thread switch.

This was the PR: #888 (comment)

[gavinking]

Oh I get it I had forgotten that worker threads are only intended for blocking operations. Carry on. :)

[DavideD]

Oh I get it I had forgotten that worker threads are only intended for blocking operations. Carry on. :)

Ah, ok :D