Iot hub java memory leak #790
Conversation
Manually check if the Future obj is cancelled. Only checking for an interrupted Thread obj is not sufficient.
Memory leak fix
|
Hi @kevinledinh, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! TTYL, AZPRBOT; |
| @@ -10,34 +10,53 @@ | |||
| import com.microsoft.azure.iothub.auth.IotHubSasToken; | |||
| import com.microsoft.azure.iothub.transport.State; | |||
| import com.microsoft.azure.iothub.transport.TransportUtils; | |||
|
|
|||
There was a problem hiding this comment.
Would it be possible to remove these extra lines added?
| public void run() throws HandlerException | ||
| { | ||
| this.reactor.setTimeout(10); | ||
| this.reactor.start(); | ||
| System.out.println("Running: " + Thread.currentThread().getName() + " id: " + Thread.currentThread().getId()); |
|
Can you please address the comments ? We can then merge changes in. |
|
@kevinledinh, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
Remove redundant variable, print statements, and white spaces as per request
Refactoring Amqp transport source file
|
Hi @prmathur-microsoft , I've committed changes as per your request. Please review and let me know if they are sufficient. Thanks. |
| @@ -623,7 +615,6 @@ private void reconnect() | |||
| { | |||
| try | |||
| { | |||
| this.close(); | |||
There was a problem hiding this comment.
I meant to say this was a good catch and we should have it .
|
Hi Kevin, Thanks for spinning a iteration. I just have two more comments before we merge it in. Thanks for your contribution. |
Re-instate close method after a reconnection failure Add Print statement to catch a non-stop executor service
Commit changes as per MS request 20160905
|
Hi @prmathur-microsoft , Changes committed as per your request. |
|
Thanks for your contribution. These changes will be merged now. |
Memory leak fix for IoT Hub Device Client Java implementation as per discussion in ticket 720 and with Microsoft Engineer Priyanka Mathur on 26th Aug 2016:
"
In IotHubReactor.java, only checking if the current Thread is interrupted is not sufficient because, from what I understand, a Thread can sometimes ignores the interrupt signal. Also, cancelling future task by reactorFuture.cancel (in AmpqIotHubConnection.java , close() ) is also not sufficient. All these methods are attempts to close a running Thread, and the Thread can (most of the time) ignore the request.
So there is a need to manually check whether the Future object is cancelled while the thread is running. I've added futureReactor.isCancelled() in run() method of IotHubReactor class to exit the thread accordingly. Also, I've used break instead of return to call reactor.stop() method before exiting run().
Please let me know if this fix is valid on your set up and whether it will be included in the next release of the IoT Hub SDK.
"
I am looking forward to hearing from the IoT Hub Team.
Best wishes,