gharris1727 commented on code in PR #15598:
URL: https://github.com/apache/kafka/pull/15598#discussion_r1695995911
##########
clients/src/main/java/org/apache/kafka/common/utils/Exit.java:
##########
@@ -119,4 +134,138 @@ public static void resetHaltProcedure() {
public static void resetShutdownHookAdder() {
shutdownHookAdder = DEFAULT_SHUTDOWN_HOOK_ADDER;
}
+
+ /**
+ * @return the default system exit behavior. Using this grants the ability
to stop the JVM at any time.
+ */
+ public static Exit system() {
+ return SystemExit.SYSTEM;
+ }
+
+ /**
+ * @return an immutable reference to exit behavior that is active at the
time this method is evaluated.
+ * <p>This may grant the ability to stop the JVM at any time if the static
exit behavior has not been changed.
+ * <p>Note: changes to the static exit behavior made after this method
will not apply to the returned object.
+ * This is used as a temporary shim between the mutable-static and
immutable-instance forms of the Exit class.
+ * This is intended to be called as early as possible on the same thread
that calls
+ * {@link #setExitProcedure(Procedure)}, {@link
#setHaltProcedure(Procedure)},
+ * or {@link #addShutdownHook(String, Runnable)} to avoid race conditions.
+ */
+ public static Exit staticContext() {
Review Comment:
@C0urante I'll reproduce your comment on the other branch here to respond:
> Is Exit.staticContext() necessary here? I understand we might use it as a
precautionary measure in case this constructor gets used in a testing context
sometime in the future, but AFAICT it'd be acceptable to use Exit.system()
right now since this doesn't appear to be used directly or indirectly in
testing code.
You are correct, that the particular call-site for staticContext() that you
commented on isn't ever called in a mocked-exit environment, so it would behave
like calling system(). The commit I showed was close to the end of the
migration, when it's possible that we could use system() in Connect.
I kept the staticContext() call because it's more defensive: if someone had
called Exit.setExitProcedure, it would pick up the override and work correctly.
Once we are sure there are no callers of setExitProcedure (e.g. no callers in
Connect, no callers in Kafka, no callers in the whole ecosystem) we can swap
staticContext() for system().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
