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

Bad design/naming in bt.runtime.BtRuntime.shutdownGracefully method #173

Open
atomashpolskiy opened this issue Jul 13, 2021 · 2 comments
Open

Comments

@atomashpolskiy
Copy link
Owner

atomashpolskiy commented Jul 13, 2021

This method is used for shutting down the lifecycle executor (of which there are currently two types: runtime startup and runtime shutdown), but uses configuration options that are supposed to control the maximum duration of individual shutdown hooks:

private void shutdownGracefully(ExecutorService executor) {
        executor.shutdown();
        try {
            long timeout = config.getShutdownHookTimeout().toMillis();
            boolean terminated = executor.awaitTermination(timeout, TimeUnit.MILLISECONDS);
            if (!terminated) {
                LOGGER.warn("Failed to shutdown executor in {} millis", timeout);
            }
        } catch (InterruptedException e) {
            // ignore
            LOGGER.warn("Interrupted while waiting for shutdown", e);
            executor.shutdownNow();
        }
    }
@atomashpolskiy
Copy link
Owner Author

Also need to move the enclosing try{} catch (InterruptedException e) {} block to the calling bt.runtime.BtRuntime.runHooks method to make sure that the exception is not accidentally swallowed with runHooks continuing to wait for something.

@yufeng-pan
Copy link

Hi atomashpolskiy, I am interested in this issue. Can I work on it?

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