[GitHub] [activemq-artemis] brusdev commented on a change in pull request #2707: ARTEMIS-2002 Proton transport objects leaked

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

brusdev commented on a change in pull request #2707: ARTEMIS-2002 Proton transport objects leaked
URL: https://github.com/apache/activemq-artemis/pull/2707#discussion_r295151846
 
 

 ##########
 File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPConnectionContext.java
 ##########
 @@ -389,13 +408,15 @@ public void onRemoteOpen(Connection connection) throws Exception {
       initialise();
 
       /*
-      * This can be null which is in effect an empty map, also we really don't need to check this for in bound connections
-      * but its here in case we add support for outbound connections.
-      * */
+       * This can be null which is in effect an empty map, also we really don't need to check this for in bound connections
+       * but its here in case we add support for outbound connections.
+       * */
       if (connection.getRemoteProperties() == null || !connection.getRemoteProperties().containsKey(CONNECTION_OPEN_FAILED)) {
          long nextKeepAliveTime = handler.tick(true);
-         if (nextKeepAliveTime != 0 && scheduledPool != null) {
-            scheduledPool.schedule(new ScheduleRunnable(), (nextKeepAliveTime - TimeUnit.NANOSECONDS.toMillis(System.nanoTime())), TimeUnit.MILLISECONDS);
+         synchronized (schedulingLock) {
+            if (nextKeepAliveTime != 0 && scheduledPool != null && !isSchedulingCancelled) {
+               scheduledFuture = scheduledPool.schedule(new ScheduleRunnable(), (nextKeepAliveTime - TimeUnit.NANOSECONDS.toMillis(System.nanoTime())), TimeUnit.MILLISECONDS);
 
 Review comment:
   It is important not to schedule another task after close, so I add isSchedulingCancelled and It needs a critical section to be synchronized with scheduledFuture.

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