hibernate-reactive and Vert.x EventBus - Session/EntityManager closes too early

[bamling]

Describe the bug
I'm trying to use the quarkus-hibernate-reactive extension with the quarkus-vertx extension and I have issues persisting data. My project looks roughly like this:

FruitResource:

@Inject
EventBus eventBus;

@POST
public Uni<Response> create(Fruit fruit) {
    if (fruit == null || fruit.getId() != null) {
        throw new WebApplicationException("Id was invalidly set on request.", 422);
    }

    return eventBus.<Void>request("create-fruit", fruit)
        .map(ignore -> Response.ok(fruit).status(201).build());
}

FruitService:

@Inject
FruitRepository fruitRepository;

@ConsumeEvent("create-fruit")
public Uni<Void> createFruit(final Fruit fruit) {
    return fruitRepository.create(fruit);
}

FruitRepository:

@Inject
Mutiny.Session mutinySession;

public Uni<Void> create(final Fruit fruit) {
    return mutinySession
        .persist(fruit)
        .chain(mutinySession::flush);
}

The exception I get is a java.lang.IllegalStateException: Session/EntityManager is closed, which I assume happens during the flush(). I guess my session closes somewhere down the line but I have no idea where and how to prevent this. I feel like this is a bug and unintended behaviour.

Expected behavior
The entity can be persisted in a reactive scenario that utilises the Vert.x event bus for messaging.

Actual behavior
See above. The exception java.lang.IllegalStateException: Session/EntityManager is closed is thrown while attempting to persist the entity.

To Reproduce
The full example can be found here: https://github.com/bamling/quarkus-hibernate-reactive-test
FruitsEndpointTest simulates the behaviour!

[ghost]

/cc @Sanne, @gsmet, @yrodiere

[CurtisBaldwinson]

In reactive programming, "things" (vaguely put) aren't typically called until a subscription is actually made. Think if you're a magazine business, you wouldn't send out any magazines until someone has actually subscribed.

Try using .persistAndFlush(). Then add .subscribeAsCompletionStage() after your persist so that it's actually executed, then return a Uni.createFrom().voidItem() if that's what you wish to return.

Or you can return a Uni, return mutinySession.persistAndFlush(fruit).call(ignore -> fruit);

[bamling]

@CurtisBaldwinson Thanks for your fast reply! I tried both of your suggested approaches but neither made a difference. The application still runs into Session/EntityManager is closed.

[gavinking]

I'm not completely sure, but the problem might be that you need a transaction. Is that right, @Sanne?

[Sanne]

Using a transaction is highly recommended, but I'm not sure if the lack of a transaction could explain this error - and at very least we should improve the error.

I've asked @DavideD to investigate. Thanks for the reproducer @bamling !

[DavideD]

I'm looking at the error and I can reproduce it. The session get closed after the event is consumed and before the query is executed. I'm still not sure why.

[DavideD]

I think this is what's happening:

1. When the event is consumed FruitRepository#findAll gets called
2. An open session is injected correctly
3. return mutinySession.createNamedQuery( "Fruits.findAll", Fruit.class ).getResults()
4. the bean destroy method is called and the session is closed

Because with Mutiny everything is lazy and non blocking, the method returns immediately even if the query hasn't been executed yet. It seems that the destroy method, and therefore session.close(), get called without making sure first that the Multi has completed. This causes the exception when the query is actually executed because the session is now closed.

Not sure exactly which layer is responsible. It seems related to CDI.

As a workaround, injecting the SessionFactory should work. Something like:

@Inject
Mutiny.SessionFactory mutinySessionFactory;

public Multi<Fruit> findAll() {
    return mutinySessionFactory.withSession( mutinySession -> 
            mutinySession.createNamedQuery( "Fruits.findAll", Fruit.class ) .getResultList()
    ).onItem().transformToMulti( list -> Multi.createFrom().iterable( list ) );
}

Probably, in this case it's easier to return a Uni<List<Fruit>> though.

[Sanne]

@mkouba , @cescoffier we'll need your insight - and your help probably as well :)

In a nutshell, it seems @Inject doesn't play well with Mutiny currently, especially with beans which need @Disposes: the injected object is available when the reactive chain is created, but when the events are actually using the bean this has been closed already.

A simple and straight forward solution would be for us to remove the capability to inject such beans, and like @DavideD suggest have users open/close the Mutiny.Session within the chain.

It would be more elegant to figure out a way to have the CDI interceptor affect the chain (effectively intercepting it "around" the right events).

[mkouba]

The problem is that a @ConsumeEvent method is executed with a new request context (unless already active) and this context is destroyed when the invocation completes, and so the Mutiny.Session can be destroyed before the Multi is completed.

I think that we should be able to fix this and implement something similar to our reactive routes handlers where the request context is destroyed when the response is diposed. In this case, we could detect that a @ConsumeEvent method returns an async type and destroy the context on async completion. We already do invoke message.reply()/failure() in that case.

[Sanne]

Sounds good @mkouba , thanks! Can I assign this issue to you?

[mkouba]

I've sent a pull request that should fix the problem. It's probably not 100% correct, i.e. it may not cover all use cases. What's changed: if a @ConsumeEvent method returns CompletionStage/Uni and the request was activated (i.e. was not active already) we destroy the request context when the asyn computation is completed.

@bamling I've tried to build your reproducer and the test passed.