joseluisll opened a new pull request, #8712: URL: https://github.com/apache/hadoop/pull/8712
### Description of PR https://issues.apache.org/jira/browse/HDFS-17972 `TestDFSClientRetries#testLeaseRenewAndDFSOutputStreamDeadLock` installs an anonymous `DFSClientFaultInjector` into the static `DFSClientFaultInjector.instance` and never restores it. The injector's `delayWhenRenewLeaseTimeout()` waits on a `CountDownLatch` with an unbounded `await()`, and that latch is counted down in exactly one place - `SleepFixedTimeAnswer.answer()` - which runs only if the mocked `NameNode.complete()` is invoked. The test's `finally` block only calls `cluster.shutdown()`. `LeaseRenewer.run()` calls the injector from inside `synchronized (this)` on the `SocketTimeoutException` abort path, so a renewer waiting there holds the `LeaseRenewer` monitor for the whole wait. The test normally passes because `complete()` releases the latch while `out1.close()` is still running. If the write pipeline stalls, `closeImpl()` never reaches `completeFile()`, `complete()` is never invoked, the latch stays at 1, and the renewer holds the monitor indefinitely. `out1.close()` then deadlocks against it on its own exit path: DFSOutputStream.close -> closeImpl -> closeThreads -> setClosed -> DFSClient.endFileLease -> LeaseRenewer.addClient (synchronized) `@Timeout(120)` does not recover this. In JUnit's default `SAME_THREAD` mode the deadline interrupts the test thread and then waits for the method to return, and a thread blocked on a monitor is not interruptible. The surefire fork therefore hangs until `forkedProcessTimeoutInSeconds` kills it, and the results for the remaining tests in the class are lost rather than reported. The static injector is also left pointing at the test's own subclass. Changes, all test-only: 1. Bound the wait to 30 seconds. This is what breaks the deadlock - the `finally` runs only after `out1.close()` returns, which is exactly where the thread is stuck, so cleanup alone cannot help. With the bound, the renewer releases the monitor and a stalled run reports one ordinary test failure instead of hanging the fork. 2. Count the latch down in the `finally`, ahead of restoring the injector and ahead of `cluster.shutdown()`, so the renewer is released promptly on the normal path. 3. Save and restore the previous injector in that same `finally`, matching the convention in `TestPread`, `TestClientProtocolForPipelineRecovery`, `TestDFSInputStream` and `TestPipelineCloseRecoveryByteArrayLeak`. Restoring alone is insufficient: it does nothing for a renewer already inside the old injector. 4. Re-assert the interrupt flag instead of `e.printStackTrace()`. No production code is touched, and the deadlock this test guards (HDFS-9294) is unaffected. ### How was this patch tested? `mvn -pl hadoop-hdfs-project/hadoop-hdfs test -Dtest=TestDFSClientRetries` - 13 tests, 0 failures, 0 errors (JDK 21, Maven 3.9.16). `testLeaseRenewAndDFSOutputStreamDeadLock` and `testLeaseRenewSocketTimeout` also pass together in that order in a single JVM. The failure was reproduced by driving the unmodified test method while withholding the DataNode's ack (`DataNodeFaultInjector.delaySendingAckToUpstream`), so `DataStreamer.waitForAckedSeqno` blocks and `complete()` is unreachable. Without this change the test method never returns after the JUnit interrupt and `DFSClientFaultInjector.get()` is still the test's own subclass; with it the method returns and the injector is restored. ### 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) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
