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

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

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

   ### Description of PR
   
   https://issues.apache.org/jira/browse/HADOOP-20011
   
   Most tests in `TestZKDelegationTokenSecretManager` and its RBF subclass 
`TestZKDelegationTokenSecretManagerImpl` release their token managers, secret 
managers and Curator clients only on the success path. When an assertion fails, 
they leak past `tearDown()`. A leaked manager's `ExpiredTokenRemover` then 
fails with `ConnectionLoss` once the `TestingServer` is closed. That breaks a 
later test, or kills the fork (HADOOP-20010), and the original failure is lost.
   
   This moves the cleanup into try/finally blocks, using null-safe destroys via 
a new `destroyIfNotNull` helper:
   
   - `TestZKDelegationTokenSecretManager`: `testMultiNodeOperationsImpl`, 
`testNodeUpAferAWhile`, `testMultiNodeCompeteForSeqNum`, 
`testRenewTokenSingleManager`, `testCancelTokenSingleManager`, 
`testStopThreads`, `testACLs`, and `testCreateNameSpaceRepeatedly`. 
`testCreateNameSpaceRepeatedly` now also resets the static curator and closes 
the Curator client it never closed before.
   - `TestZKDelegationTokenSecretManagerImpl`: the three `*WithoutWatch` tests.
   
   Not touched, because other changes already cover them:
   - `testNodesLoadedAfterRestart` (HADOOP-20009, merged).
   - `testCreatingParentContainersIfNeeded` and `testMultipleInit` 
(HADOOP-19966, #8688).
   
   This PR merges cleanly with #8688 and with #8805 (HADOOP-20010).
   
   Test-only change. Most of the diff is re-indentation; reviewing with 
whitespace hidden (`?w=1`) is easier.
   
   ### How was this patch tested?
   
   JDK 21, on current trunk:
   
   - `mvn -pl hadoop-common-project/hadoop-common test 
-Dtest=TestDelegationToken,TestZKDelegationTokenSecretManager`: 31 tests, 0 
failures.
   - `mvn -pl hadoop-hdfs-project/hadoop-hdfs-rbf test 
-Dtest=TestZKDelegationTokenSecretManagerImpl`: 15 tests, 0 failures.
   
   ### 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)
   




> Release token managers and Curator clients in finally blocks in 
> TestZKDelegationTokenSecretManager
> --------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-20011
>                 URL: https://issues.apache.org/jira/browse/HADOOP-20011
>             Project: Hadoop Common
>          Issue Type: Test
>          Components: security
>            Reporter: Jose Luis López
>            Assignee: Jose Luis López
>            Priority: Major
>
> Most tests in {{TestZKDelegationTokenSecretManager}} and its RBF subclass 
> {{TestZKDelegationTokenSecretManagerImpl}} release their 
> {{DelegationTokenManager}} instances, secret managers and Curator clients 
> only on the success path. When an assertion fails, these resources leak.
> A leaked manager's {{ExpiredTokenRemover}} keeps running after {{tearDown()}} 
> closes the ZooKeeper {{TestingServer}}. It then fails with {{ConnectionLoss}} 
> while a later test is running, so the next test fails or the surefire fork is 
> killed, and the original failure is lost (see HADOOP-20010). A leaked 
> thread-local curator set via {{ZKDelegationTokenSecretManager.setCurator}} 
> can also leak into later tests.
> *Proposed fix:* move cleanup into try/finally blocks, with null-safe 
> destroys, in:
> * {{TestZKDelegationTokenSecretManager}}: {{testMultiNodeOperationsImpl}}, 
> {{testNodeUpAferAWhile}}, {{testMultiNodeCompeteForSeqNum}}, 
> {{testRenewTokenSingleManager}}, {{testCancelTokenSingleManager}}, 
> {{testStopThreads}}, {{testACLs}} and {{testCreateNameSpaceRepeatedly}}.
> * {{TestZKDelegationTokenSecretManagerImpl}}: the three {{*WithoutWatch}} 
> tests.
> Already covered elsewhere, so not touched here:
> * {{testNodesLoadedAfterRestart}} (HADOOP-20009).
> * {{testCreatingParentContainersIfNeeded}} and {{testMultipleInit}} 
> (HADOOP-19966, [PR #8688|https://github.com/apache/hadoop/pull/8688]).
> Test-only change. Most of the diff is re-indentation; review with whitespace 
> ignored.



--
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