changeset 210:0a92d6b1b97f

[commands] Explicitly set agent socket session timeout. Reviewed-by: jkang Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-July/024246.html
author Severin Gehwolf <sgehwolf@redhat.com>
date Fri, 21 Jul 2017 09:46:31 +0200
parents aeb1467ddd67
children a94c8f8c4c2b
files services/commands/src/main/java/com/redhat/thermostat/service/commands/socket/CommandChannelAgentSocket.java services/commands/src/test/java/com/redhat/thermostat/service/commands/socket/CommandChannelAgentSocketTest.java
diffstat 2 files changed, 71 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/services/commands/src/main/java/com/redhat/thermostat/service/commands/socket/CommandChannelAgentSocket.java	Fri Jul 21 09:45:37 2017 -0400
+++ b/services/commands/src/main/java/com/redhat/thermostat/service/commands/socket/CommandChannelAgentSocket.java	Fri Jul 21 09:46:31 2017 +0200
@@ -39,6 +39,7 @@
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
+import java.util.concurrent.TimeUnit;
 
 import javax.websocket.PongMessage;
 import javax.websocket.Session;
@@ -51,13 +52,20 @@
 
 class CommandChannelAgentSocket extends CommandChannelSocket {
 
+    private static final long SOCKET_SESSION_IDLE_TIMEOUT = TimeUnit.MINUTES.toMillis(10);
     private static final String UNKNOWN_PAYLOAD = "UNKNOWN";
     private static final String AGENT_PROVIDER_PREFIX = "thermostat-commands-provider-";
-    private final long socketTimeout;
 
     CommandChannelAgentSocket(String id, Session session) {
         super(id, session);
-        this.socketTimeout = session.getMaxIdleTimeout();
+        // Be sure to have the socket timeout the same as on the
+        // agent side. Otherwise the agent socket might
+        // time out. Note that the time period upon
+        // which we will send ping messages is relative to the
+        // configured socket timeout. It must be strictly less than
+        // the used socket timeout on both sides. This ensures that
+        // the agent socket does not close on us and is kept alive.
+        session.setMaxIdleTimeout(SOCKET_SESSION_IDLE_TIMEOUT);
     }
 
     @Override
@@ -70,14 +78,14 @@
         //        connects in that window it will get an error back,
         //        believing that the agent it wants to talk to has not
         //        connected.
-        AgentSocketsRegistry reg = AgentSocketsRegistry.getInstance(socketTimeout);
+        AgentSocketsRegistry reg = AgentSocketsRegistry.getInstance(SOCKET_SESSION_IDLE_TIMEOUT);
         reg.addSocket(agentId, this.session);
     }
 
     @Override
     public void onClose(int closeCode, String reason) {
         super.onClose(closeCode, reason);
-        AgentSocketsRegistry reg = AgentSocketsRegistry.getInstance(socketTimeout);
+        AgentSocketsRegistry reg = AgentSocketsRegistry.getInstance(SOCKET_SESSION_IDLE_TIMEOUT);
         reg.removeSocket(agentId);
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/services/commands/src/test/java/com/redhat/thermostat/service/commands/socket/CommandChannelAgentSocketTest.java	Fri Jul 21 09:46:31 2017 +0200
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2012-2017 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.service.commands.socket;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import javax.websocket.Session;
+
+import org.junit.Test;
+
+public class CommandChannelAgentSocketTest {
+
+    /**
+     * The agent side of the channel sets it to 10 minutes,
+     * this is to verify they are kept in sync.
+     */
+    @Test
+    public void verifySessionTimeoutSetCorrectly() {
+        Session session = mock(Session.class);
+        long expectedTimeout = 600_000; // 10 minutes in ms
+        new CommandChannelAgentSocket("foo-id", session);
+        verify(session).setMaxIdleTimeout(expectedTimeout);
+    }
+}