[
https://issues.apache.org/jira/browse/HADOOP-20011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18127639#comment-18127639
]
ASF GitHub Bot commented on HADOOP-20011:
-----------------------------------------
joseluisll commented on PR #8806:
URL: https://github.com/apache/hadoop/pull/8806#issuecomment-6203462616
Pushed one change: the RBF
`TestZKDelegationTokenSecretManagerImpl.tearDown()`
now resets the Curator in a `finally` block, so a failing
`super.tearDown()` doesn't leak a STOPPED Curator into the next test.
This moved here from #8805 (HADOOP-20010), where it no longer belonged.
Coordination with related PRs:
- `testCreatingParentContainersIfNeeded` and `testMultipleInit` are
intentionally not touched here. #8688 (HADOOP-19966) already moves their
cleanup into `finally` blocks, and changing them here too would conflict.
- With #8688, a failed `init()` cleans up inside `startThreads()`, and the
`finally` blocks here then call `destroy()` again. That second stop is
harmless and keeps this PR correct on its own.
- #8805 makes `TestZKDelegationTokenSecretManager` fail a test if a leaked
remover thread calls `ExitUtil.terminate`, so any leak that slips past
these `finally` blocks shows up as a test failure instead of a crashed
fork.
All three PRs merge cleanly in any order.
> 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
> Labels: pull-request-available
>
> 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]
