[GitHub] merlimat commented on a change in pull request #1405: Timeout task for batchMessageAndSendTask is not cancelled when close

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

merlimat commented on a change in pull request #1405: Timeout task for batchMessageAndSendTask is not cancelled when close
URL: https://github.com/apache/incubator-pulsar/pull/1405#discussion_r175497968
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
 ##########
 @@ -1150,7 +1157,8 @@ public void run(Timeout timeout) throws Exception {
                 batchMessageAndSend();
             }
             // schedule the next batch message task
-            client.timer().newTimeout(this, conf.getBatchingMaxPublishDelayMicros(), TimeUnit.MICROSECONDS);
+            batchMessageAndSendTimeout = client.timer()
 
 Review comment:
   @zhaijack I think there might still be a small chance to get the exception here. 
   
   Example: 
    * The timeout handler is running
    * We close the producer & cancel the timer
    * We still try to do `newTimeout()` here
   
   I think this should be fixed by putting the `newTimeout()` call inside the above `synchronized (ProducerImpl.this) {}` block. 
   
   That should ensure that either the timer is fired (and then cancelled) or it's not retriggered.
   
   We should also check the producer state in the same sync block. If it's closing/closed we need to ignore the send batch timer and not schedule next timeout.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services