Shutdown task priority

[Marko Bekhta]

Hi all,

tl;dr;

While exploring ways to make the order of shutting down the extensions more predictable, I ended up having a priority queue for shutdown tasks. Since this affects more than just a few extensions, let's discuss it here for a broader audience to be aware.

Details:

The problem: in some cases, Hibernate Search may require a fully operational Hibernate ORM extension to gracefully shut down, as it needs to store data in the database. For that to happen, the Hibernate ORM shutdown (which will trigger closing the Hibernate Search extension) must occur while the CDI container hasn't yet triggered its shutdown, and more importantly, the DB is still accessible.

See the original discussion at [1] and code changes at [2].

I've explored various approaches (app shutdown event listeners, CDI bean destroyers, shutdown tasks), and prioritising shutdown tasks is the most suitable solution so far.

The way I see it, we have the following "significant" points in the shutdown timeline:

• App shutdown event
• CDI container shutdown
• Core extensions shutdown

While everything else happens in between.

It all started with a simple int priority that one could pass with the shutdown task to add it to a prioritised queue. However, as Yoann suggested that if we miss something now, it may be problematic to change the priority ranges later, I've changed things a bit so that instead of passing a simple int, there's a `Priority` class expected, and a set of factory methods are available to get the task into a particular segment of the shutdown timeline.
This shutdown priority is not explicitly tied to the order in which the extensions are "started". However, it is implicitly tied, as shutdown tasks are added by extensions during startup. Using the default priority puts them in an order such that they should shut down in reverse compared to their start.

If we discover any other significant points in this "timeline", we can add them to that factory interface and base other priorities on it.

Factory methods take +/- values, where a negative value means add the task to the "end-of-the-priority-range minus value-passed-to-factory-method", so that it would be easier to add the tasks either at the end or start of the priority range. 

As this would affect more than one extension, what are your thoughts on this?

[1] https://github.com/quarkusio/quarkus/issues/34547
[2] https://github.com/quarkusio/quarkus/pull/46357

[David Lloyd]

I think using a priority is inherently problematic, since task start and stop already has a clear and specific dependency order. By definition, a task cannot start until after its dependencies have started; at this point order does not (and should not) matter because we want to start things in parallel when possible. Likewise, a task cannot stop until after its dependents have stopped, otherwise it may not be possible for some things to cleanly shut down.

We do have something of a problem where our shutdown tasks are more or less flattened into a linear list, which is non-ideal. But I don't think we should break the invariant of never stopping tasks before their dependents are stopped.

If we have a case where something is stopping before its dependents are stopping - which it sounds like may be happening here - then it's the dependency relationship that is broken and must be fixed. Adding in numeric priorities in any form is something we have explicitly avoided as a point of design from day one.

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/44a67e90-3a90-4882-a1fe-188f46bcb668n%40googlegroups.com.

--

- DML • he/him

[Yoann Rodiere]

If I remember correctly though, the whole of CDI is one big shutdown task, relative order of shutdown between beans being completely undefined...

Worse, there can be cyclic dependencies between beans, so CDI dependencies themselves cannot really be used to automate shutdown order. So, in some cases at least, we'll need to be explicit?

I'll grant you that we could be explicit by providing additional metadata ("this bean depends on that one, but for real") instead of using priorities, so that we're able to build a dependency... tree I guess? But that would only work if the dependency problem is within CDI and we don't have things like a shutdown task depending on a CDI bean, itself depending on another shutdown task -- which I'm not entirely sure is the case, but I'll let Marko confirm/deny.

Yoann Rodière

Principal Software Engineer, Middleware

Hibernate team architect

Red Hat

To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CANghgrSspvhqh1xgQbpHQfEge8mcw-D_mRV6RLAjpL_oCg134w%40mail.gmail.com.

[Holly Cummins]

Could it work to re-use the hard thinking of the BuildStep mechanism? Something like an TeardownStep? It’s not quite so clean because build steps consume and produce things, but teardown steps would consume and produce the absence of things. Things like “ThereIsntAnArchiveAnymoreBuildItem” would just make developer’s heads hurt. :) 

But I agree with David that priority is a bit brittle as a mechanism, because you need to know what phase the things you depend on have chosen, and also what phase the things that depend on you have chosen, and if anyone changes their phase it could ripple through the whole chain. 

Another complexity is that “restart” is heavily overloaded in Quarkus-land, so I suspect “shutdown” is also overloaded. I keep intending to write a blog post about it, but in our code comments we use restart to mean “the process was restarted”, “the ‘application’ was restarted,” “the curated application was restarted,” “continuous testing was restarted,” and a few other things as well. It would be nice (and I know here I’m expanding the scope a lot beyond what Marko had hoped) to have a stricter and clearer set of semantics for what a ‘restart’ is. Those could then be mirrored into a set of types of shutdown. A chaining model might make it easier to reason about all the different shutdowns by eliminating the need for words. :) 

<shutdown.png>

It all started with a simple int priority that one could pass with the shutdown task to add it to a prioritised queue. However, as Yoann suggested that if we miss something now, it may be problematic to change the priority ranges later, I've changed things a bit so that instead of passing a simple int, there's a `Priority` class expected, and a set of factory methods are available to get the task into a particular segment of the shutdown timeline.
This shutdown priority is not explicitly tied to the order in which the extensions are "started". However, it is implicitly tied, as shutdown tasks are added by extensions during startup. Using the default priority puts them in an order such that they should shut down in reverse compared to their start.

If we discover any other significant points in this "timeline", we can add them to that factory interface and base other priorities on it.

Factory methods take +/- values, where a negative value means add the task to the "end-of-the-priority-range minus value-passed-to-factory-method", so that it would be easier to add the tasks either at the end or start of the priority range. 

As this would affect more than one extension, what are your thoughts on this?

[1] https://github.com/quarkusio/quarkus/issues/34547
[2] https://github.com/quarkusio/quarkus/pull/46357

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/44a67e90-3a90-4882-a1fe-188f46bcb668n%40googlegroups.com.

--

- DML • he/him

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CANghgrSspvhqh1xgQbpHQfEge8mcw-D_mRV6RLAjpL_oCg134w%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.

To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CAEqagpqQ_Ps98DQ0Sk_bWDhE5TeMRWPZ707KVgVCWnQaUtYXWg%40mail.gmail.com.

[Ladislav Thon]

Dne po 16. 6. 2025 18:06 uživatel 'Yoann Rodiere' via Quarkus Development mailing list <quark...@googlegroups.com> napsal:

If I remember correctly though, the whole of CDI is one big shutdown task, relative order of shutdown between beans being completely undefined...

Beans do not shut down, beans are destroyed by the container when it's shutting down, and the `@PreDestroy` callbacks, exactly for the reason you mention, must not assume that other beans still exist. In other words, `@PreDestroy` callbacks must be _local_.

For non-local shutdown processing, purely on the CDI front, beans should observe `ShutdownEvent`, because that event is fired when the CDI container is still up and all beans are still available. (Note that numeric priorities already exist there.)

But from the original e-mail, it seems to me that CDI beans and their destruction callbacks / `ShutdownEvent` observers are not used here, or at least are not where the problem is?

LT

To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CAEqagpqQ_Ps98DQ0Sk_bWDhE5TeMRWPZ707KVgVCWnQaUtYXWg%40mail.gmail.com.

[Marko Bekhta]

Thanks, everyone. I also found your comment, Ladislav, on GitHub about this [1]. So, yes, that's exactly why we'd want to go away from this CDI-assumption here in the ORM extension [2]. 

When I explored the option of observing the `ShutdownEvent`, we ended up discarding it. It's been a while since then, so I don't remember all the reasons, but the ones I do were:

• This event can also be observed by the application code (users) -- we'd want to keep the ORM available to the users in those shutdown event observers, i.e. we cannot destroy (or make it unusable by shutting down ORM) that bean just yet.
• If something goes wrong and the app fails before starting (i.e. before the point where it registers the shutdown task that fires the shutdown event for observers to consume), we still would want to correctly clean up after ourselves if possible (in the ORM extensions)

That's how we arrived at shutdown tasks 🙂. Currently, there are two methods to register the tasks:

public interface ShutdownContext {
    void addShutdownTask(Runnable runnable);
    // these are executed after all the ones add via addShutdownTask in the reverse order from which they were added
    void addLastShutdownTask(Runnable runnable);

}

Which already brings some notion of a shutdown priority in a sense... And this idea somewhat builds on top of that. 

>> By definition, a task cannot start until after its dependencies have started; at this point order does not (and should not) matter because we want to start things in parallel when possible. Likewise, a task cannot stop until after its dependents have stopped, otherwise it may not be possible for some things to cleanly shut down.

I agree with that, and that's also what Yoann was saying somewhere in one of the PRs. But then again, `addLastShutdownTask`, maybe it misled me 🙂. 

>> If we have a case where something is stopping before its dependents are stopping - which it sounds like may be happening here - then it's the dependency relationship that is broken and must be fixed. Adding in numeric priorities in any form is something we have explicitly avoided as a point of design from day one.

That is currently so, but it's because it is built on the assumption about the CDI rather than the use of the `io.quarkus.runtime.ShutdownContext`... so maybe setting the priority explicitly is actually redundant, let me run some tests to make sure things are as I'd hope them to be... And if so, maybe we should look into that "addLastShutdownTask" whether it can be dropped and replaced by adding the dependencies ... (but that would be a separate task).

> Could it work to re-use the hard thinking of the BuildStep mechanism? Something like an TeardownStep? It’s not quite so clean because build steps consume and produce things, but teardown steps would consume and produce the absence of things. Things like “ThereIsntAnArchiveAnymoreBuildItem” would just make developer’s heads hurt. :) 

Hmm, so the tear-down step would produce the shutdown task, but it would still need some input items to get the dependencies, right? It might be nice to have it so it is easier to spot the shutdown task for the extension .... but yeah, I don't know.

And yes 🙂 (about the head).

Let me run some tests, and I'll get back to you.

[1] https://github.com/quarkusio/quarkus/issues/34547#issuecomment-2288096186

[2] https://github.com/quarkusio/quarkus/blob/7e64426809af1da2432efea2116a2cf51f0cb425/extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java#L172-L188

[3] https://github.com/quarkusio/quarkus/blob/7e64426809af1da2432efea2116a2cf51f0cb425/core/runtime/src/main/java/io/quarkus/runtime/ShutdownContext.java#L15-L18

You received this message because you are subscribed to a topic in the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/quarkus-dev/CMq7i4EZMXY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to quarkus-dev...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CALbocOmx%2BCrioKgoGyaXJXqQEC8d%3DDzY6TTJkPT49nqy5wmDoQ%40mail.gmail.com.

[Holly Cummins]

While we’re (sort of) on the subject, I will never not be confused by code like  
 this:

shutdownListenerBuildItemBuildProducer.produce(new ShutdownListenerBuildItem(
        recorder.initializeLogging(logRuntimeConfig, logBuildTimeConfig, discoveredLogComponents,
                categoryMinLevelDefaults.content, alwaysEnableLogStream,
                streamingDevUiLogHandler, handlers, namedHandlers,
                possibleConsoleFormatters, possibleFileFormatters, possibleSyslogFormatters,
                possibleSocketFormatters,
                possibleSupplier, launchModeBuildItem.getLaunchMode(), true)));

When I’m debugging something, I just skip over that kind of line, because I’m not worried about what’s happening at shutdown, right? 
Nope, wrong. I just missed a big piece of startup logic. 
To me, a pattern like this would be much clearer, because it doesn’t bury the ‘real’ action inside shutdown logic.

recorder.initializeLogging(logRuntimeConfig, logBuildTimeConfig, discoveredLogComponents,

                categoryMinLevelDefaults.content, alwaysEnableLogStream,
                streamingDevUiLogHandler, handlers, namedHandlers,
                possibleConsoleFormatters, possibleFileFormatters, possibleSyslogFormatters,
                possibleSocketFormatters,
                possibleSupplier, launchModeBuildItem.getLaunchMode(), true, shutdownListenerBuildItemBuildProducer)));

To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CACYzyevjTGDihdFBrmF60K%3DmxpJUs%2BCKQVZfn%2BaVejXueCmHwg%40mail.gmail.com.

[Marko Bekhta]

Hi all,

As promised, I'm getting back to you after doing some more testing. And it looks ... we can do it without introducing the priority 🥳. I believe I have found a suitable spot to register the ORM's shutdown task, ensuring that it still has access to all the CDI beans, while also executing it after the shutdown event is processed. 

Thanks for the discussion and pointers; I'm glad that at least it sparked other ideas, even if the priority ended up being unnecessary 🙂.

Have a nice day,

Marko

To view this discussion visit https://groups.google.com/d/msgid/quarkus-dev/CA3CF71E-DC38-48AF-9E55-A3D00C14126D%40redhat.com.