[GitHub] [pulsar] eolivelli commented on a change in pull request #10159: [Client][Java] Close deadLetterProducer in Java consumer client (prevent potential leak)

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #10159:
URL: https://github.com/apache/pulsar/pull/10159#discussion_r609359089



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -938,6 +938,21 @@ private void closeConsumerTasks() {
         if (possibleSendToDeadLetterTopicMessages != null) {
             possibleSendToDeadLetterTopicMessages.clear();
         }
+
+        if (deadLetterProducer != null) {
+            createProducerLock.writeLock().lock();
+            try {
+                if (deadLetterProducer != null) {
+                    deadLetterProducer.thenApplyAsync(Producer::closeAsync);
+                    deadLetterProducer = null;
+                }
+            } catch (Exception e) {
+                log.error("Error closing deadLetterProducer for topic: {}", deadLetterPolicy.getDeadLetterTopic(), e);

Review comment:
       should we handle InterruptedException here ?
   

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java
##########
@@ -938,6 +938,21 @@ private void closeConsumerTasks() {
         if (possibleSendToDeadLetterTopicMessages != null) {
             possibleSendToDeadLetterTopicMessages.clear();
         }
+
+        if (deadLetterProducer != null) {
+            try {
+                createProducerLock.writeLock().lock();
+                if (deadLetterProducer != null) {
+                    deadLetterProducer.thenApplyAsync(Producer::closeAsync);

Review comment:
       +1 for blocking until the close operation finishes




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org