Closed ylexus closed 1 month ago
eitch
commented
1 month ago I'm not quite sure I'm following. Pi4j only stops everything, when shutdown is called. Maybe you are just calling it too soon? Do you have a more detailed example of how you are managing pi4j's lifecycle?
ylexus
commented
1 month ago @eitch
Pi4j only stops everything, when shutdown is called.
So intuitively it should, but unfortunately this is not the case. As you can see, the DefaultRuntime installs a global JVM shutdown hook and shuts the context down when that hook fires. As a result:
My point is that the global shutdown hook that you can't turn off is not a good idea to have in a library, because the application that uses the library loses full control of the shutdown sequence.
Hope this is clear.
ylexus
commented
1 month ago The workaround for this is to register a pi4j shutdown listener and block the pi4j shutdown until all the components that use it have shut down (https://github.com/ylexus/jiotty/blob/master/jiotty-connector-rpi/src/main/java/net/yudichev/jiotty/connector/rpigpio/Pi4jContextProvider.java). It's rather elaborate; disabling pi4j shutdown hook would be a much preferred solution.
final class Pi4jContextProvider extends BaseLifecycleComponent implements Provider<Context> {
private static final Logger logger = LoggerFactory.getLogger(Pi4jContextProvider.class);
private static final int SHUTDOWN_TIMEOUT_SECONDS = 15;
private final CountDownLatch preShutdownLatch = new CountDownLatch(1);
private final CountDownLatch postShutdownLatch = new CountDownLatch(1);
private volatile boolean pi4jShuttingDown;
private Context gpio;
@Override
public Context get() {
return whenStartedAndNotLifecycling(() -> gpio);
}
@Override
public void doStart() {
gpio = Pi4J.newAutoContext();
// TODO this is a workaround for https://github.com/Pi4J/pi4j-v2/issues/371 - remove when fixed
gpio.addListener(new ShutdownListener() {
@Override
public void beforeShutdown(ShutdownEvent event) {
pi4jShuttingDown = true;
logger.debug("Blocking pi4j shutdown until this component is closed");
asUnchecked(() -> {
var stopTriggered = preShutdownLatch.await(SHUTDOWN_TIMEOUT_SECONDS, SECONDS);
if (stopTriggered) {
logger.debug("Stopping - pi4j shutdown released");
} else {
logger.warn("Timed out waiting for this component to be stopped");
}
});
}
@Override
public void onShutdown(ShutdownEvent event) {
postShutdownLatch.countDown();
}
});
}
@Override
protected void doStop() {
logger.debug("Releasing pi4j shutdown gate and shutting down pi4j");
preShutdownLatch.countDown();
// best effort prevent double shutdown call and awaiting successful shutdown, will work if the pi4j shutdown hook fires first
if (pi4jShuttingDown) {
// pi4's own shutdown hook is doing the job - wait for shutdown to finish
logger.debug("Waiting for pi4j to shut down");
asUnchecked(() -> {
var stopped = postShutdownLatch.await(SHUTDOWN_TIMEOUT_SECONDS, SECONDS);
if (stopped) {
logger.debug("pi4j shut down");
} else {
logger.warn("Timed out waiting for pi4 to shut down");
}
});
} else {
gpio.shutdown(); // does it synchronously
}
}
}Hi @ylexus
How about this: https://github.com/Pi4J/pi4j-v2/pull/372
You can now simply disable the shutdown hook using this:
Pi4J.newContextBuilder().autoDetect().disableShutdownHook().build();I never really realized, that pi4j add this shutdown hook, all my test apps, which are simple console apps, etc. add their own shutdown hook to shutdown pi4j. Apps should really control the life cycle, not the library, i totally agree.
FDelporte
commented
1 month ago not the library
what should be the default then?
ylexus
commented
1 month ago Hi @ylexus
How about this: https://github.com/Pi4J/pi4j-v2/pull/372
You can now simply disable the shutdown hook using this:
Pi4J.newContextBuilder().autoDetect().disableShutdownHook().build();
Yes that would totally work, thanks. This is exactly what I asked.
eitch
commented
1 month ago Fixed in next release
Related somewhat to #213. I have an application that properly manages component dependencies, including the order of initialisation and shutdown. pi4j context is just another component in the app that is initialised before it's used by other components, and is shut down via
Context.shutdown()(which shuts down theRuntime) just after all the components that use it have already shut down.However,
DefaultRuntimeadds a shutdown hook that is executed concurrently with my application shutdown sequence and obliterates the context before the components that use pi4j are stopped, causing exceptions (example is below, but it does not matter).I think a component based library that has clear start/stop points (which pi4j does) should not unconditionally start shutting itself down. I agree it's useful for apps don't care about shutdown sequences or exceptions, but for more organised apps like mine it would be very useful to make the shutdown hook optional.
I can raise a PR, possibly by adding a property that disables it so that a user could do:
, but please recommend the best way of doing this.