[ 
https://issues.apache.org/jira/browse/HADOOP-20010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18124998#comment-18124998
 ] 

ASF GitHub Bot commented on HADOOP-20010:
-----------------------------------------

joseluisll opened a new pull request, #8805:
URL: https://github.com/apache/hadoop/pull/8805

   ### Description of PR
   
   https://issues.apache.org/jira/browse/HADOOP-20010
   
   `AbstractDelegationTokenSecretManager.ExpiredTokenRemover.run()` catches 
every `Throwable` and calls `Runtime.getRuntime().exit(-1)`. Tests can't 
intercept that, and it can also fire while the secret manager is being stopped.
   
   - **Use `ExitUtil.terminate(-1, t)`** instead of 
`Runtime.getRuntime().exit(-1)`, so tests that call 
`ExitUtil.disableSystemExit()` get an `ExitException` instead of a dead JVM.
   - **Don't exit while stopping.** `stopThreads()` clears `running` and then 
interrupts the thread. With a ZooKeeper-backed store, the interrupt can surface 
as a `RuntimeException`. If `running` is already false, the exception is logged 
and the thread returns.
   - **`TestZKDelegationTokenSecretManager`** disables system exit in `setup()` 
and fails in `tearDown()` if `ExitUtil.terminate` was called. Without this 
check, a remover thread from a leaked manager would kill the surefire fork 
after the ZooKeeper `TestingServer` closes, and the original failure would be 
lost. The RBF subclass's `tearDown()` now resets the static curator in 
`finally`, so the base `tearDown()` can throw without skipping that reset.
   
   ### How was this patch tested?
   
   JDK 21, one fresh fork per test class:
   
   - `mvn -pl hadoop-common-project/hadoop-common test 
-Dtest=TestDelegationToken,TestZKDelegationTokenSecretManager`: 33 tests, 0 
failures. This includes two new tests in `TestDelegationToken`:
     - `testExpiredTokenRemoverFailureTerminates`: a remover failure while 
running calls `ExitUtil.terminate(-1, ...)`.
     - `testExpiredTokenRemoverFailureWhileStoppingDoesNotTerminate`: a failure 
raised while `stopThreads()` is interrupting the thread does not.
   - `mvn -pl hadoop-hdfs-project/hadoop-hdfs-rbf test 
-Dtest=TestZKDelegationTokenSecretManagerImpl`: 15 tests, 0 failures, with 
HADOOP-20009 applied.
   
   Before HADOOP-20009 was merged, a failing `testNodesLoadedAfterRestart` 
leaked a manager. About 48 s later, its remover hit `ConnectionLoss` during the 
next test. With this patch that showed up as a `tearDown()` failure. On trunk 
the same path calls `Runtime.exit(-1)` and kills the fork.
   
   ### For code changes:
   
   - [x] Does the title of this PR start with the corresponding JIRA issue id 
(e.g. 'HADOOP-17799. Your PR title ...')?
   - [ ] Object storage: Have the integration tests been executed and the 
endpoint declared according to the connector-specific documentation?
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, 
`NOTICE-binary` files?
   
   ### AI Tooling
   
   Contains content generated by Claude Code.
   
   - [x] The PR includes the phrase "Contains content generated by <tool>" 
where <tool> is the name of the AI tool used.
   - [x] My use of AI contributions follows the ASF legal policy 
https://www.apache.org/legal/generative-tooling.html
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   




> ExpiredTokenRemover should use ExitUtil instead of Runtime.exit
> ---------------------------------------------------------------
>
>                 Key: HADOOP-20010
>                 URL: https://issues.apache.org/jira/browse/HADOOP-20010
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: security
>            Reporter: Jose Luis López
>            Assignee: Jose Luis López
>            Priority: Major
>
> {{AbstractDelegationTokenSecretManager.ExpiredTokenRemover.run()}} catches 
> every {{Throwable}} and calls {{Runtime.getRuntime().exit(-1)}}. This has two 
> problems:
> # *It bypasses {{ExitUtil}}.* Tests that call 
> {{ExitUtil.disableSystemExit()}} can't intercept it, so the JVM is killed 
> instead. In {{TestZKDelegationTokenSecretManager}} and 
> {{TestZKDelegationTokenSecretManagerImpl}}, a token manager leaked by a 
> failing test keeps its remover thread running after {{tearDown()}} closes the 
> ZooKeeper {{TestingServer}}. On its next pass the thread hits 
> {{ConnectionLoss}}, which is wrapped in a {{RuntimeException}}, and kills the 
> surefire fork. The original test failure is lost.
> # *It exits during a normal shutdown.* {{stopThreads()}} sets {{running = 
> false}} and then interrupts the thread. With a ZooKeeper-backed store, the 
> interrupt can surface as a {{RuntimeException}} instead of an 
> {{InterruptedException}}, and that terminates the process while it is being 
> stopped.
> *Proposed fix:*
> * Call {{ExitUtil.terminate(-1, t)}} instead of 
> {{Runtime.getRuntime().exit(-1)}}.
> * If {{running}} is already false, log the exception and return instead of 
> exiting.
> * Make {{TestZKDelegationTokenSecretManager}} disable system exit and fail in 
> {{tearDown()}} if {{ExitUtil.terminate}} was called.
> *Tests:* two new tests in {{TestDelegationToken}}:
> * A remover failure while running calls {{ExitUtil.terminate(-1, ...)}}.
> * A failure while {{stopThreads()}} is running does not.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to