From 95adb7bfbfc69b3e9f3538cc5d6f7c6a577d30ee Mon Sep 17 00:00:00 2001 From: Gaurav Narula Date: Mon, 20 May 2024 20:10:08 +0100 Subject: [PATCH] MINOR: ensure KafkaServerTestHarness::tearDown is always invoked (#15996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An exception thrown while closing the client instances in `IntegrationTestHarness::tearDown` may result in `KafkaServerTestHarness::tearDown` not being invoked. This would result in thread leaks of the broker and controller threads spawned in the failing test. An example of this is the [CI run](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15994/1/tests) for #15994 where `Build / JDK 8 and Scala 2.12 / testCoordinatorFailover(String, String).quorum=kraft+kip848.groupProtocol=consumer – kafka.api.PlaintextConsumerTest` failing results in `consumers.foreach(_.close(Duration.ZERO))` in `IntegrationTestHarness::tearDown` throwing an exception. A side effect of this is it poisons Gradle test runner JVM and prevents tests in other unrelated classes from executing as `@BeforeAll` check in QuorumTestHarness would cause them to fail immediately. This PR encloses the client closure in try-finally to ensure`KafkaServerTestHarness::tearDown` is always invoked. Reviewers: Nikhil Ramakrishnan , Igor Soarez , Chia-Ping Tsai --- .../kafka/api/IntegrationTestHarness.scala | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala b/core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala index 6d40d65cc26..e1f549c4e53 100644 --- a/core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala +++ b/core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala @@ -220,16 +220,18 @@ abstract class IntegrationTestHarness extends KafkaServerTestHarness { @AfterEach override def tearDown(): Unit = { - producers.foreach(_.close(Duration.ZERO)) - consumers.foreach(_.wakeup()) - consumers.foreach(_.close(Duration.ZERO)) - adminClients.foreach(_.close(Duration.ZERO)) + try { + producers.foreach(_.close(Duration.ZERO)) + consumers.foreach(_.wakeup()) + consumers.foreach(_.close(Duration.ZERO)) + adminClients.foreach(_.close(Duration.ZERO)) - producers.clear() - consumers.clear() - adminClients.clear() - - super.tearDown() + producers.clear() + consumers.clear() + adminClients.clear() + } finally { + super.tearDown() + } } }