From 049b9888619409c410d9356566815c7732dbbbe7 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 25 Feb 2015 22:15:28 +0200 Subject: [PATCH] Don't actively disconnect when the client already has disconnected (#15330) Change-Id: I26e53f6b07eaccc785bda547e454fa185ad952df --- .../AtmospherePushConnection.java | 27 ++++++++++++++----- .../server/communication/PushHandler.java | 16 +++++------ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java index 0819a24ee9..ab45fcfe89 100644 --- a/server/src/com/vaadin/server/communication/AtmospherePushConnection.java +++ b/server/src/com/vaadin/server/communication/AtmospherePushConnection.java @@ -275,12 +275,10 @@ public class AtmospherePushConnection implements PushConnection { assert isConnected(); if (resource.isResumed()) { - // Calling disconnect may end up invoking it again via - // resource.resume and PushHandler.onResume. Bail out here if - // the resource is already resumed; this is a bit hacky and should - // be implemented in a better way in 7.2. - resource = null; - state = State.DISCONNECTED; + // This can happen for long polling because of + // http://dev.vaadin.com/ticket/16919 + // Once that is fixed, this should never happen + connectionLost(); return; } @@ -307,8 +305,23 @@ public class AtmospherePushConnection implements PushConnection { getLogger() .log(Level.INFO, "Error when closing push connection", e); } + connectionLost(); + } + + /** + * Called when the connection to the client has been lost. + * + * @since + */ + public void connectionLost() { resource = null; - state = State.DISCONNECTED; + if (state == State.CONNECTED) { + // Guard against connectionLost being (incorrectly) called when + // state is PUSH_PENDING or RESPONSE_PENDING + // (http://dev.vaadin.com/ticket/16919) + state = State.DISCONNECTED; + } + } /** diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java index 7e7183193a..22eee70aa0 100644 --- a/server/src/com/vaadin/server/communication/PushHandler.java +++ b/server/src/com/vaadin/server/communication/PushHandler.java @@ -64,7 +64,7 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { throws IOException { super.onStateChange(event); if (event.isCancelled() || event.isResumedOnTimeout()) { - disconnect(event); + connectionLost(event); } } @@ -329,17 +329,17 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { public void onDisconnect(AtmosphereResourceEvent event) { // Log event on trace level super.onDisconnect(event); - disconnect(event); + connectionLost(event); } @Override public void onThrowable(AtmosphereResourceEvent event) { getLogger().log(Level.SEVERE, "Exception in push connection", event.throwable()); - disconnect(event); + connectionLost(event); } - private void disconnect(AtmosphereResourceEvent event) { + private void connectionLost(AtmosphereResourceEvent event) { // We don't want to use callWithUi here, as it assumes there's a client // request active and does requestStart and requestEnd among other // things. @@ -425,12 +425,8 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter { "Connection unexpectedly closed for resource {0} with transport {1}", new Object[] { id, resource.transport() }); } - if (pushConnection.isConnected()) { - // disconnect() assumes the push connection is connected but - // this method can currently be called more than once during - // disconnect, depending on the situation - pushConnection.disconnect(); - } + + pushConnection.connectionLost(); } } catch (final Exception e) { -- 2.39.5