From d84abce76074a75e830c3bf270e61c6c28608a4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez=20L=C3=B3pez?= Date: Wed, 2 Sep 2026 16:06:01 +0000 Subject: [PATCH 1/2] HDFS-17972. TestDFSClientRetries: restore the fault injector and release its latch when the deadlock test dies testLeaseRenewAndDFSOutputStreamDeadLock installs an anonymous DFSClientFaultInjector into the static DFSClientFaultInjector.instance and never restores it. That injector's delayWhenRenewLeaseTimeout() waits on a CountDownLatch that is only counted down from SleepFixedTimeAnswer.answer(), i.e. only if the mocked NameNode.complete() actually runs. The test's finally block only shut the cluster down. LeaseRenewer.run() calls delayWhenRenewLeaseTimeout() from inside synchronized (this), on the SocketTimeoutException abort path, so a renewer waiting in that injector holds the LeaseRenewer monitor for as long as it waits. The test works because complete() releases the latch while out1.close() is still running, before close() needs that monitor again. If the write pipeline stalls, out1.close() never reaches completeFile(), so complete() is never invoked and the latch stays at 1. The renewer then holds the LeaseRenewer monitor indefinitely, and out1.close() deadlocks against it on its own way out: closeImpl() -> closeThreads() -> setClosed() -> DFSClient.endFileLease() -> LeaseRenewer.addClient() (synchronized) @Timeout does not rescue this. In SAME_THREAD mode JUnit interrupts the test thread at the deadline and then waits for the method to return, and a thread blocked on a monitor is not interruptible, so the method never returns. The surefire fork hangs until forkedProcessTimeoutInSeconds kills it, which loses the results for the rest of the class. Fix the test isolation: - Bound the wait to 30 seconds. This is the part that actually breaks the deadlock: the finally block below runs only after out1.close() returns, so it cannot help a thread that is stuck inside close(). With the bound, the renewer releases the monitor, close() completes, and a stalled run fails as one ordinary test failure instead of hanging the fork. Also re-assert the interrupt flag rather than printing the trace. - Count the latch down in the finally, before restoring the injector and before cluster.shutdown(), so the renewer is released immediately on the normal path instead of waiting out the bound. - Save the previous injector and restore it in that same finally, so the static does not stay pointed at this test's subclass, matching the convention used by TestPread, TestClientProtocolForPipelineRecovery, TestDFSInputStream and TestPipelineCloseRecoveryByteArrayLeak. Restoring alone would not be enough: it does nothing for a renewer already waiting inside the old injector. Verified by simulating the stall so that complete() is never reached: without the bound the test hangs in out1.close() on the renewer monitor and never returns; with it the test completes, the injector is restored and the latch released. testLeaseRenewAndDFSOutputStreamDeadLock and testLeaseRenewSocketTimeout both still pass in that order, in 10.3s and 4.1s, so the normal path is unaffected. This is a test-only change; the deadlock the test covers is unaffected. Co-Authored-By: Claude Opus 5 --- .../apache/hadoop/hdfs/TestDFSClientRetries.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java index 8eb2f588228f06..7ac4dd6ad11998 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java @@ -1279,12 +1279,15 @@ public void testDFSClientConfigurationLocateFollowingBlock() @Timeout(value = 120) public void testLeaseRenewAndDFSOutputStreamDeadLock() throws Exception { CountDownLatch testLatch = new CountDownLatch(1); + DFSClientFaultInjector oldInjector = DFSClientFaultInjector.get(); DFSClientFaultInjector.set(new DFSClientFaultInjector() { public void delayWhenRenewLeaseTimeout() { try { - testLatch.await(); + // Bounded, so a renewer thread cannot be parked here forever if this + // test dies without running its finally block. + testLatch.await(30, TimeUnit.SECONDS); } catch (InterruptedException e) { - e.printStackTrace(); + Thread.currentThread().interrupt(); } } }); @@ -1323,6 +1326,12 @@ public void delayWhenRenewLeaseTimeout() { out1.close(); } finally { + // Release any lease renewer thread parked in the injector above. If this + // test dies before NameNode.complete() runs, the latch is never counted + // down by SleepFixedTimeAnswer, and restoring the injector alone does not + // free a thread that is already inside the old one. + testLatch.countDown(); + DFSClientFaultInjector.set(oldInjector); cluster.shutdown(); } } From 57605c6fc830376ca346334abf0c1c602c384d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Luis=20L=C3=B3pez=20L=C3=B3pez?= Date: Fri, 4 Sep 2026 10:19:16 +0200 Subject: [PATCH 2/2] Update hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java Co-authored-by: Cheng Pan --- .../java/org/apache/hadoop/hdfs/TestDFSClientRetries.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java index 7ac4dd6ad11998..36dd6c546f1b46 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSClientRetries.java @@ -1283,8 +1283,8 @@ public void testLeaseRenewAndDFSOutputStreamDeadLock() throws Exception { DFSClientFaultInjector.set(new DFSClientFaultInjector() { public void delayWhenRenewLeaseTimeout() { try { - // Bounded, so a renewer thread cannot be parked here forever if this - // test dies without running its finally block. + // Bounded, so a stalled out1.close() cannot leave the renewer + // holding the LeaseRenewer monitor forever. testLatch.await(30, TimeUnit.SECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt();