Make ZpeUpdPolLoader ScheduledExecutorService thread daemon

[thonra]

Description

Background

In our services, we frequently call the static method AuthZpeClient.allowAccess to determine whether access is allowed. The underlying ZpeClient which it relies on is initialized in AuthZpeClient's static block, so callers typically don’t pay attention to its initialization lifecycle.

ZpeClient (implemented by ZpeUpdater) depends on the ZpeUpdPolLoader instance. ZpeUpdPolLoader automatically creates a ScheduledExecutorService, which currently does not shut down automatically on process exit; it requires an explicit close() call from the caller. Recently, we observed that this executor remained alive during shutdown, which blocked Tomcat’s graceful exit.

Pain point

The pain point is that when using the static allowAccess API, client code performs no initialization and is therefore unlikely to realize it must invoke close() to cascade the shutdown and release the underlying ScheduledExecutorService. This leads to negative impact on the service shutdown path.

Proposal

To prevent threads created by ZpeUpdPolLoader from blocking Tomcat’s graceful shutdown, set them as daemon threads and give them explicit names to make troubleshooting easier.

Contribution Checklist:

• The pull request does not introduce any breaking changes
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

#3085

[gemini-code-assist]

Summary of Changes

Hello @thonra, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a critical improvement to the ZpeUpdPolLoader component by integrating a JVM shutdown hook. This ensures that the ScheduledExecutorService managed by the loader is gracefully terminated upon process exit, resolving a previous issue where unclosed executors could impede server shutdown processes like Tomcat's. The change centralizes resource management within the component responsible for creating the executor, making its lifecycle transparent to client code and enhancing overall system stability during shutdown.

Highlights

• Automatic Resource Cleanup: Implements a JVM shutdown hook in ZpeUpdPolLoader to ensure the ScheduledExecutorService is automatically closed when the process terminates.
• Improved Shutdown Reliability: Addresses an issue where the ScheduledExecutorService in ZpeUpdPolLoader could block graceful Tomcat shutdowns due to unreleased resources.
• Enhanced close() Method: The close() method in ZpeUpdPolLoader is made idempotent, handles potential exceptions during executor shutdown, and safely removes the shutdown hook if close() is called during normal runtime.
• Comprehensive Unit Tests: Adds new unit tests to verify the idempotency of the close() method, the correct invocation of close() by the shutdown hook, and robust handling of IllegalStateException during hook removal.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a JVM shutdown hook to ZpeUpdPolLoader to ensure the ScheduledExecutorService is properly shut down, preventing issues with graceful application shutdown. The implementation is solid, using an AtomicBoolean for idempotency and correctly handling the lifecycle of the shutdown hook to prevent memory leaks. The accompanying tests are comprehensive and cover the new logic well. I have one suggestion to improve the robustness of the close() method in an edge case.

[mauhiz]

suggestion : make threads daemon, and prefix the thread names with a known substring

[thonra]

/gemini summary

[gemini-code-assist]

Summary of Changes

This pull request addresses a resource management issue where the ScheduledExecutorService in ZpeUpdPolLoader could prevent graceful application shutdown by keeping non-daemon threads alive. The change modifies the executor's thread creation to mark them as daemon threads and provides them with explicit names, ensuring they do not block the JVM exit and improving troubleshooting capabilities.

Highlights

• Graceful Shutdown: The ScheduledExecutorService within ZpeUpdPolLoader is now configured to use daemon threads, preventing it from blocking graceful application shutdown processes like Tomcat's.
• Improved Debuggability: Threads created by the ScheduledExecutorService are now assigned explicit, descriptive names (e.g., ZpeUpdPolLoader-scheduler-) to facilitate easier troubleshooting.

Changelog

• clients/java/zpe/src/main/java/com/yahoo/athenz/zpe/ZpeUpdPolLoader.java
  • Modified the initialization of the scheduledExecutorSvc to use a custom ThreadFactory. This factory now creates threads that are set as daemon and are given a specific naming convention: ZpeUpdPolLoader-scheduler- followed by a unique hash code.

Activity

• Reviewer mauhiz suggested making the threads daemon and prefixing their names, which has been implemented in this PR.
• Author thonra requested a summary from gemini-code-assist[bot].

[thonra]

/gemini review

[gemini-code-assist]

Code Review

This pull request addresses an issue where a ScheduledExecutorService in ZpeUpdPolLoader was preventing graceful shutdown of services like Tomcat. The change correctly configures the executor's threads as daemon threads using a ThreadFactory. This ensures they no longer block process exit. The change also assigns an explicit name to the thread, which is helpful for debugging. My review includes a suggestion to simplify the thread naming for better readability.

[thonra]

@TakuyaMatsu san,
Since you introduced the changes to the ZPE close methods, I think you’re likely familiar with the ZPE thread exit issue. I’d really appreciate it if you could take a quick look at this PR. 🙇