From 705f986462cb02d014e2f3f2537c229d125ceb83 Mon Sep 17 00:00:00 2001 From: Michael Mimeault Date: Mon, 16 Apr 2018 10:55:19 -0400 Subject: [PATCH 1/2] Safely call close on the websocket client with synchronized calls --- .../java/com/parse/OkHttp3SocketClientFactory.java | 12 +++++++----- .../java/com/parse/ParseLiveQueryClientImpl.java | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java b/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java index 71c7c28..bc2e91e 100644 --- a/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java +++ b/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java @@ -35,7 +35,7 @@ static class OkHttp3WebSocketClient implements WebSocketClient { private final WebSocketClientCallback webSocketClientCallback; private WebSocket webSocket; - private State state = State.NONE; + private volatile State state = State.NONE; private final OkHttpClient client; private final String url; private final int STATUS_CODE = 1000; @@ -73,7 +73,7 @@ public void onFailure(okhttp3.WebSocket webSocket, Throwable t, Response respons }; private OkHttp3WebSocketClient(OkHttpClient okHttpClient, - WebSocketClientCallback webSocketClientCallback, URI hostUrl) { + WebSocketClientCallback webSocketClientCallback, URI hostUrl) { client = okHttpClient; this.webSocketClientCallback = webSocketClientCallback; url = hostUrl.toString(); @@ -95,7 +95,9 @@ public synchronized void open() { @Override public synchronized void close() { setState(State.DISCONNECTING); - webSocket.close(STATUS_CODE, CLOSING_MSG); + if (webSocket != null) { + webSocket.close(STATUS_CODE, CLOSING_MSG); + } } @Override @@ -110,10 +112,10 @@ public State getState() { return state; } - private synchronized void setState(State newState) { + private void setState(State newState) { this.state = newState; this.webSocketClientCallback.stateChanged(); } } -} \ No newline at end of file +} diff --git a/ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java b/ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java index a6d1161..466a6f9 100644 --- a/ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java +++ b/ParseLiveQuery/src/main/java/com/parse/ParseLiveQueryClientImpl.java @@ -1,9 +1,7 @@ package com.parse; import android.util.Log; -import bolts.Continuation; -import bolts.Task; -import okhttp3.OkHttpClient; + import org.json.JSONException; import org.json.JSONObject; @@ -17,6 +15,10 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import bolts.Continuation; +import bolts.Task; +import okhttp3.OkHttpClient; + import static com.parse.Parse.checkInit; /* package */ class ParseLiveQueryClientImpl implements ParseLiveQueryClient { @@ -142,7 +144,7 @@ public void unsubscribe(final ParseQuery query, final } @Override - public void reconnect() { + public synchronized void reconnect() { if (webSocketClient != null) { webSocketClient.close(); } @@ -154,7 +156,7 @@ public void reconnect() { } @Override - public void disconnect() { + public synchronized void disconnect() { if (webSocketClient != null) { webSocketClient.close(); webSocketClient = null; From d07f3924d09e333e15be4b9a24d119388801f171 Mon Sep 17 00:00:00 2001 From: Michael Mimeault Date: Mon, 16 Apr 2018 11:23:38 -0400 Subject: [PATCH 2/2] Added a test and more synchronized logic --- .../com/parse/OkHttp3SocketClientFactory.java | 10 ++-- .../com/parse/TestOkHttpClientFactory.java | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 ParseLiveQuery/src/test/java/com/parse/TestOkHttpClientFactory.java diff --git a/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java b/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java index bc2e91e..9dd8769 100644 --- a/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java +++ b/ParseLiveQuery/src/main/java/com/parse/OkHttp3SocketClientFactory.java @@ -94,14 +94,14 @@ public synchronized void open() { @Override public synchronized void close() { - setState(State.DISCONNECTING); - if (webSocket != null) { + if (State.NONE != state) { + setState(State.DISCONNECTING); webSocket.close(STATUS_CODE, CLOSING_MSG); } } @Override - public void send(String message) { + public synchronized void send(String message) { if (state == State.CONNECTED) { webSocket.send(message); } @@ -113,7 +113,9 @@ public State getState() { } private void setState(State newState) { - this.state = newState; + synchronized (this) { + this.state = newState; + } this.webSocketClientCallback.stateChanged(); } } diff --git a/ParseLiveQuery/src/test/java/com/parse/TestOkHttpClientFactory.java b/ParseLiveQuery/src/test/java/com/parse/TestOkHttpClientFactory.java new file mode 100644 index 0000000..1012db9 --- /dev/null +++ b/ParseLiveQuery/src/test/java/com/parse/TestOkHttpClientFactory.java @@ -0,0 +1,49 @@ +package com.parse; + +import junit.framework.Assert; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import java.net.URI; + +import okhttp3.OkHttpClient; + +import static com.parse.WebSocketClient.State.NONE; + +@RunWith(MockitoJUnitRunner.class) +public class TestOkHttpClientFactory { + + @Mock + private OkHttpClient okHttpClientMock; + + @Mock + private WebSocketClient.WebSocketClientCallback webSocketClientCallbackMock; + + private OkHttp3SocketClientFactory okHttp3SocketClientFactory; + private WebSocketClient webSocketClient; + + @Before + public void setUp() throws Exception { + okHttp3SocketClientFactory = new OkHttp3SocketClientFactory(okHttpClientMock); + webSocketClient = okHttp3SocketClientFactory.createInstance(webSocketClientCallbackMock, new URI("http://www.test.com")); + } + + @After + public void tearDown() { + webSocketClient.close(); + } + + @Test + public void testClientCloseWithoutOpenShouldBeNoOp() { + Assert.assertEquals(NONE, webSocketClient.getState()); + webSocketClient.close(); + webSocketClient.send("test"); + Assert.assertEquals(NONE, webSocketClient.getState()); + } + +}