HADOOP-20010. Use ExitUtil instead of Runtime.exit in ExpiredTokenRemover. by joseluisll · Pull Request #8805 · apache/hadoop

joseluisll · 2026-10-08T11:42:49Z

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 and the throwable is not an Error, it is logged at INFO and the thread returns. Errors (e.g. OOM) still terminate.
TestZKDelegationTokenSecretManager adds its own @BeforeEach that disables system exit and an @AfterEach that fails the test if ExitUtil.terminate was called, then resets the recorded exit. They are separate lifecycle methods, so the check runs even when tearDown() fails. Without this, a remover thread from a leaked manager would kill the surefire fork after the ZooKeeper TestingServer closes, and the original failure would be lost. A leaked remover that terminates after the check is reported by the next test.

The RBF TestZKDelegationTokenSecretManagerImpl.tearDown() change (resetting the static curator in finally) is handled separately in HADOOP-20011. Switching SecurityUtil.doAsLoginUserOrFatal to ExitUtil is tracked in HADOOP-20022.

How was this patch tested?

JDK 21, one fresh fork per test class:

mvn -pl hadoop-common-project/hadoop-common test -Dtest=TestDelegationToken,TestZKDelegationTokenSecretManager: 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 shows up as a test failure. On trunk the same path calls Runtime.exit(-1) and kills the fork.

For code changes:

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?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

AI Tooling

Contains content generated by Claude Code.

The PR includes the phrase "Contains content generated by " where is the name of the AI tool used.
My use of AI contributions follows the ASF legal policy https://www.apache.org/legal/generative-tooling.html

🤖 Generated with Claude Code

joseluisll · 2026-10-12T21:36:44Z

@slfan1989 @zhtttylz would you have time to review this PR? This goes also with related PR #8806

smengcl

Thanks @joseluisll for the patch. Claude generated some review comments. ptal

smengcl

Thanks @joseluisll . lgtm

…over. The expired token remover thread called Runtime.getRuntime().exit(-1) on any unexpected Throwable. Tests could not intercept it, so a token manager leaked past tearDown() killed the surefire fork when its ZooKeeper server went away. Use ExitUtil.terminate() so tests can disable System.exit, and don't terminate when the exception arrives after stopThreads() has cleared the running flag and interrupted the thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…enRemover tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Let Errors raised while stopping still terminate, and log the exception at INFO without a stack trace on a normal stop. - Always stop the manager and reset the ExitUtil state in testExpiredTokenRemoverFailureWhileStoppingDoesNotTerminate. - Check and reset the ExitUtil state in a finally block in TestZKDelegationTokenSecretManager#tearDown, and use fail(). - Use assertThat(...).hasMessage(...) for the cause message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

joseluisll · 2026-10-15T17:40:28Z

CI errors are covered by HADOOP-19966: token cache restart race (testNodesLoadedAfterRestart)
#8688 and HADOOP-19997: TestSSLHttpServerMTLS intermittent SocketException #8783 .

Disable System.exit and check the recorded termination in dedicated @BeforeEach/@AfterEach methods instead of editing setup() and tearDown(), so this change does not overlap HADOOP-19966 (apache#8688), which also edits them. JUnit runs every @AfterEach method even if tearDown() fails, so the ExitUtil state is still always checked and reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The ExitUtil check no longer runs in tearDown(), so wrapping super.tearDown() in try/finally is plain resource cleanup and belongs with the other finally-block fixes in HADOOP-20011. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

joseluisll · 2026-10-15T22:20:05Z

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 HADOOP-20011. Release token managers and Curator clients in finally blocks in TestZKDelegationTokenSecretManager. #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.

hadoop-yetus · 2026-10-16T01:40:23Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 26s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 3 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	1m 58s		Maven dependency ordering for branch
+1 💚	mvninstall	29m 30s		trunk passed
+1 💚	compile	9m 33s		trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	compile	9m 41s		trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	checkstyle	3m 11s		trunk passed
+1 💚	mvnsite	2m 12s		trunk passed
+1 💚	javadoc	1m 37s		trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javadoc	1m 37s		trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	spotbugs	3m 17s		trunk passed
+1 💚	shadedclient	19m 6s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 19s		Maven dependency ordering for patch
+1 💚	mvninstall	1m 15s		the patch passed
+1 💚	compile	9m 14s		the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javac	9m 14s		the patch passed
+1 💚	compile	9m 51s		the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	javac	9m 51s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	3m 37s		the patch passed
+1 💚	mvnsite	2m 24s		the patch passed
+1 💚	javadoc	1m 33s		the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javadoc	1m 33s		the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	spotbugs	3m 39s		the patch passed
+1 💚	shadedclient	18m 29s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 49s		hadoop-common in the patch passed.
+1 💚	unit	43m 45s		hadoop-hdfs-rbf in the patch passed.
+1 💚	asflicense	0m 43s		The patch does not generate ASF License warnings.
		202m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/9/artifact/out/Dockerfile
GITHUB PR	#8805
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 1cf2acc7620c 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `19d1ada`
Default Java	Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
Multi-JDK versions	/usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/9/testReport/
Max. process+thread count	3402 (vs. ulimit of 10000)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/9/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.7
Powered by	Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

joseluisll · 2026-10-22T14:13:38Z

@smengcl @slfan1989 This PR is ready for review.

smengcl

Thanks @joseluisll . lgtm. One optional nit. Pls update PR description to match the code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

hadoop-yetus · 2026-10-23T00:32:32Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 37s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 3 new or modified test files.
			_ trunk Compile Tests _
+0 🆗	mvndep	2m 47s		Maven dependency ordering for branch
+1 💚	mvninstall	44m 36s		trunk passed
+1 💚	compile	16m 27s		trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	compile	16m 45s		trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	checkstyle	5m 43s		trunk passed
+1 💚	mvnsite	3m 43s		trunk passed
+1 💚	javadoc	2m 38s		trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javadoc	2m 36s		trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	spotbugs	5m 40s		trunk passed
+1 💚	shadedclient	31m 5s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 28s		Maven dependency ordering for patch
+1 💚	mvninstall	2m 19s		the patch passed
+1 💚	compile	15m 38s		the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javac	15m 38s		the patch passed
+1 💚	compile	16m 45s		the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	javac	16m 45s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	5m 27s		the patch passed
+1 💚	mvnsite	3m 38s		the patch passed
+1 💚	javadoc	2m 37s		the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu
+1 💚	javadoc	2m 37s		the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
+1 💚	spotbugs	6m 2s		the patch passed
+1 💚	shadedclient	31m 16s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	23m 34s		hadoop-common in the patch passed.
+1 💚	unit	44m 57s		hadoop-hdfs-rbf in the patch passed.
+1 💚	asflicense	1m 19s		The patch does not generate ASF License warnings.
		295m 17s

Subsystem	Report/Notes
Docker	ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/10/artifact/out/Dockerfile
GITHUB PR	#8805
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 0c2faf7153f0 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `e0479b6`
Default Java	Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
Multi-JDK versions	/usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/10/testReport/
Max. process+thread count	3773 (vs. ulimit of 10000)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-rbf U: .
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8805/10/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.7
Powered by	Apache Yetus 0.14.1 https://yetus.apache.org

This message was automatically generated.

joseluisll · 2026-10-23T05:48:11Z

Addresed the review comments, ready to be merged.

smengcl · 2026-10-23T08:10:25Z

Thanks @joseluisll for the patch.

joseluisll mentioned this pull request Oct 8, 2026

HADOOP-20011. Release token managers and Curator clients in finally blocks in TestZKDelegationTokenSecretManager. #8806

Open

3 of 6 tasks

github-actions Bot added HDFS trunk Common RBF labels Oct 8, 2026

This comment was marked as outdated.

Sign in to view

smengcl reviewed Oct 15, 2026

View reviewed changes

joseluisll force-pushed the HADOOP-20010 branch from d553f0b to 08364ed Compare October 15, 2026 06:41

smengcl approved these changes Oct 15, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

joseluisll and others added 3 commits October 15, 2026 10:26

HADOOP-20010. Share the failing secret manager between the ExpiredTok…

324633b

…enRemover tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

joseluisll force-pushed the HADOOP-20010 branch from 08364ed to dc7ab3b Compare October 15, 2026 10:29

This comment was marked as outdated.

Sign in to view

joseluisll mentioned this pull request Oct 15, 2026

HADOOP-19966. ZKDelegationTokenSecretManager startup cache load races with CuratorCache initialization #8688

Open

3 of 6 tasks

joseluisll and others added 2 commits October 16, 2026 00:04

HADOOP-20010. Re-enable System.exit after the ExpiredTokenRemover tests.

c658283

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions Bot removed HDFS RBF labels Oct 15, 2026

This comment was marked as outdated.

Sign in to view

joseluisll requested a review from smengcl October 22, 2026 14:13

smengcl approved these changes Oct 22, 2026

View reviewed changes

Comment thread ...op-common/src/test/java/org/apache/hadoop/security/token/delegation/TestDelegationToken.java Outdated

HADOOP-20010. Report the ExitException in the stopping test.

e0479b6

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

joseluisll requested a review from smengcl October 23, 2026 05:47

smengcl approved these changes Oct 23, 2026

View reviewed changes

smengcl merged commit e96f042 into apache:trunk Oct 23, 2026
6 checks passed

Conversation

joseluisll commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

How was this patch tested?

For code changes:

AI Tooling

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

joseluisll commented Oct 12, 2026

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

joseluisll commented Oct 15, 2026

Uh oh!

joseluisll commented Oct 15, 2026

Uh oh!

This comment was marked as outdated.

hadoop-yetus commented Oct 16, 2026

Uh oh!

joseluisll commented Oct 22, 2026

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hadoop-yetus commented Oct 23, 2026

Uh oh!

joseluisll commented Oct 23, 2026

Uh oh!

Uh oh!

smengcl commented Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joseluisll commented Oct 8, 2026 •

edited

Loading