[
https://issues.apache.org/jira/browse/HADOOP-20010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127638#comment-18127638
]
ASF GitHub Bot commented on HADOOP-20010:
-----------------------------------------
joseluisll commented on PR #8805:
URL: https://github.com/apache/hadoop/pull/8805#issuecomment-6203461837
Pushed two small changes:
- Both ExpiredTokenRemover tests in TestDelegationToken now call
`ExitUtil.enableSystemExit()` in their `finally` blocks, so the
disabled-exit state doesn't leak into later tests in the class.
`TestZKDelegationTokenSecretManager` intentionally keeps exit disabled
for the whole class: re-enabling it after each test would let a leaked
remover thread kill the fork again, which is what this PR fixes.
- Dropped the try/finally around `super.tearDown()` in the RBF
`TestZKDelegationTokenSecretManagerImpl`. It was only needed while the
ExitUtil check lived in `tearDown()`. Since that check moved to its own
`@AfterEach`, it's plain resource cleanup, so I moved it to #8806
(HADOOP-20011).
I checked this together with #8806 and #8688 (HADOOP-19966). All three
merge cleanly in any order. With #8688, a failed `startThreads()` calls
`stopThreads()` before the remover thread exists, so it never reaches the
new `!running` path here.
> 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
> Labels: pull-request-available
>
> {{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]
