[GitHub] [pulsar] merlimat opened a new pull request #4427: Fixed NPE when closing batch during a reconnection

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

merlimat opened a new pull request #4427: Fixed NPE when closing batch during a reconnection
URL: https://github.com/apache/pulsar/pull/4427
 
 
   ### Motivation
   
   There is a race condition when closing a batch and the producer is getting disconnected. This has been seen in some of the tests run and can be reproduced by running this particular test many times in a loop.
   
   ```
   2019-05-31\T\16:32:49.647 [ERROR] testProducerReconnect(org.apache.pulsar.client.api.ClientErrorsTest)  Time elapsed: 0.034 s  <<< FAILURE!
   org.apache.pulsar.client.api.PulsarClientException: java.lang.NullPointerException
   	at org.apache.pulsar.client.impl.ProducerImpl.batchMessageAndSend(ProducerImpl.java:1354)
   	at org.apache.pulsar.client.impl.ProducerImpl.triggerFlush(ProducerImpl.java:1282)
   	at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:82)
   	at org.apache.pulsar.client.impl.TypedMessageBuilderImpl.send(TypedMessageBuilderImpl.java:61)
   	at org.apache.pulsar.client.impl.ProducerBase.send(ProducerBase.java:52)
   	at org.apache.pulsar.client.api.ClientErrorsTest.testProducerReconnect(ClientErrorsTest.java:600)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.lang.reflect.Method.invoke(Method.java:498)
   	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:124)
   	at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:54)
   	at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:44)
   	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
   	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
   	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
   	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
   	at java.lang.Thread.run(Thread.java:748)
   Caused by: java.lang.NullPointerException
   	at org.apache.pulsar.client.impl.ProducerImpl.batchMessageAndSend(ProducerImpl.java:1329)
   	... 17 more
   ```
   
   The problem is that we're checking `isConnected()` and then using the `cnx()` which can return null if the producer is disconnected. Rather, we should first cache the reference to the `ClientCnx` instance and then double check if we're connected.  
   
   The same pattern is already applied for the non-batching scenario: 
   
   https://github.com/apache/pulsar/blob/ba24d73b5f77c42f832cd9d2720dc0aba388f909/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L399-L405
   
   

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


With regards,
Apache Git Services