gharris1727 opened a new pull request, #15598: URL: https://github.com/apache/kafka/pull/15598
This adds a thread-safe alternative to the current Exit implementation, which doesn't make use of static mutable fields. This solves a number of problems: 1. Thread isolation: Two tests can inject exit procedures concurrently, without conflicting with one-another 2. The test-only methods for manipulating the state of Exit are present on the main class, and so could be called in a non-test environment. 3. When fatal procedures are intercepted by Exit, shutdown hooks are delayed until the real JVM exit, possibly leaking resources/files across tests. 4. When fatal procedures are intercepted by Exit, those exceptions can go un-noticed, and may not cause the test to fail. 5. When shutdown hooks are intercepted by Exit, they are dropped and never executed 6. It isn't clear to code calling exit() that an exception might be thrown instead when the procedures are mocked. The Java and Scala Exit classes don't use any instance methods, so I was able to directly add instance methods to the existing classes. This should prevent the need for a large rename later, such as appears necessary with the other options I could think of: * Add it as a different class SafeExit: large amount of code churn when Exit is removed, silly name * Moving "Exit" to "UnsafeExit" now: large amount of code churn up-front, immediate merge conflicts * Add it to a different package as "utils.temp.Exit": moderate amount of code churn when Exit is removed, harder to review Because static and instance methods cannot share signatures, I had to perturb the names slightly. I think the new names are acceptable, and shouldn't cause any additional code churn. * exit to exitOrThrow * halt to haltOrThrow * addShutdownHook to addShutdownRunnable In the interest of not testing tests, I've omitted tests for these changes. The existing ExitTest is testing the mocking methods, which are not part of the main API now, just the MockExit API. ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
