From 79d6f346b68fc1f20656de057562ea4efc44b421 Mon Sep 17 00:00:00 2001 From: Andy Prock Date: Thu, 19 Nov 2015 09:59:37 -0800 Subject: [PATCH] simplified the android code UdpSocketClient holds onto the only DatagramSocket/MulticastSocket reference. UdpSocketClient throws IlllegalStateException if send/addMembership are called on unbound sockets. UdpReceiverTask takes it's values as a Pair parameter to execute on. --- .../com/tradle/react/UdpReceiverTask.java | 57 +++++----------- .../com/tradle/react/UdpSocketClient.java | 68 ++++++++++--------- .../java/com/tradle/react/UdpSockets.java | 50 ++++++++------ 3 files changed, 83 insertions(+), 92 deletions(-) diff --git a/android/src/main/java/com/tradle/react/UdpReceiverTask.java b/android/src/main/java/com/tradle/react/UdpReceiverTask.java index c2e897b..1eb2091 100644 --- a/android/src/main/java/com/tradle/react/UdpReceiverTask.java +++ b/android/src/main/java/com/tradle/react/UdpReceiverTask.java @@ -5,59 +5,44 @@ * Created by Andy Prock on 9/24/15. */ - package com.tradle.react; +package com.tradle.react; - import android.os.AsyncTask; - import android.util.Base64; +import android.os.AsyncTask; +import android.util.Base64; +import android.util.Pair; - import java.io.IOException; - import java.lang.ref.WeakReference; - import java.net.DatagramPacket; - import java.net.DatagramSocket; - import java.net.InetAddress; +import java.io.IOException; +import java.net.DatagramPacket; +import java.net.DatagramSocket; +import java.net.InetAddress; /** * This is a specialized AsyncTask that receives data from a socket in the background, and * notifies it's listener when data is received. This is not threadsafe, the listener * should handle synchronicity. */ -public class UdpReceiverTask extends AsyncTask { +public class UdpReceiverTask extends AsyncTask, Void, Void> { private static final String TAG = "UdpReceiverTask"; private static final int MAX_UDP_DATAGRAM_LEN = 1024; - private DatagramSocket mSocket; - private WeakReference mReceiverListener; - - /** - * An {@link AsyncTask} that blocks to receive data from a socket. - * Received data is sent via the {@link OnDataReceivedListener} - */ - public UdpReceiverTask(DatagramSocket socket, UdpReceiverTask.OnDataReceivedListener - receiverListener) { - this.mSocket = socket; - this.mReceiverListener = new WeakReference<>(receiverListener); - } - - /** - * Returns the UdpReceiverTask's DatagramChannel. - */ - public DatagramSocket getSocket() { - return mSocket; - } - /** * An infinite loop to block and read data from the socket. */ @Override - protected Void doInBackground(Void... a) { - OnDataReceivedListener receiverListener = mReceiverListener.get(); + protected Void doInBackground(Pair... params) { + if (params.length > 1) { + throw new IllegalArgumentException("This task is only for a single socket/listener pair."); + } + + DatagramSocket socket = params[0].first; + OnDataReceivedListener receiverListener = params[0].second; final byte[] buffer = new byte[MAX_UDP_DATAGRAM_LEN]; DatagramPacket packet = new DatagramPacket(buffer, buffer.length); while (!isCancelled()) { try { - mSocket.receive(packet); + socket.receive(packet); final InetAddress address = packet.getAddress(); final String base64Data = Base64.encodeToString(packet.getData(), packet.getOffset(), @@ -79,14 +64,6 @@ public class UdpReceiverTask extends AsyncTask { return null; } - /** - * Close if cancelled. - */ - @Override - protected void onCancelled() { -// mSocket.close(); - } - /** * Listener interface for receive events. */ diff --git a/android/src/main/java/com/tradle/react/UdpSocketClient.java b/android/src/main/java/com/tradle/react/UdpSocketClient.java index 3d160bf..177d166 100644 --- a/android/src/main/java/com/tradle/react/UdpSocketClient.java +++ b/android/src/main/java/com/tradle/react/UdpSocketClient.java @@ -10,6 +10,7 @@ package com.tradle.react; import android.os.AsyncTask; import android.support.annotation.Nullable; import android.util.Base64; +import android.util.Pair; import com.facebook.react.bridge.Callback; @@ -47,11 +48,11 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList } /** - * Checks to see if client is receiving multicast packets. - * @return boolean true if receiving multicast packets, else false. + * Checks to see if client part of a multi-cast group. + * @return boolean true IF the socket is part of a multi-cast group. */ public boolean isMulticast() { - return (mReceiverTask != null && mReceiverTask.getSocket() instanceof MulticastSocket); + return (mSocket != null && mSocket instanceof MulticastSocket); } /** @@ -69,7 +70,7 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList public void bind(Integer port, @Nullable String address) throws IOException { mSocket = new DatagramSocket(null); - mReceiverTask = new UdpReceiverTask(mSocket, this); + mReceiverTask = new UdpReceiverTask(); SocketAddress socketAddress; if (address != null) { @@ -82,7 +83,8 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList mSocket.bind(socketAddress); // begin listening for data in the background - mReceiverTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); + mReceiverTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, + new Pair(mSocket, this)); } /** @@ -92,18 +94,30 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList * @param address the multicast group to join * @throws UnknownHostException * @throws IOException + * @throws IllegalStateException if socket is not bound. */ - public void addMembership(String address) throws UnknownHostException, IOException { - final int port = mReceiverTask.getSocket().getLocalPort(); - mReceiverTask.cancel(true); + public void addMembership(String address) throws UnknownHostException, IOException, IllegalStateException { + if (null == mSocket || !mSocket.isBound()) { + throw new IllegalStateException("Socket is not bound."); + } - final MulticastSocket socket = new MulticastSocket(port); - socket.joinGroup(InetAddress.getByName(address)); + if (!(mSocket instanceof MulticastSocket)) { + // cancel the current receiver task + if (mReceiverTask != null) { + mReceiverTask.cancel(true); + } - mReceiverTask = new UdpReceiverTask(socket, this); + // tear down the DatagramSocket, and rebuild as a MulticastSocket + final int port = mSocket.getLocalPort(); + mSocket.close(); + mSocket = new MulticastSocket(port); - // begin listening for data in the background - mReceiverTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); + // begin listening for data in the background + mReceiverTask.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, + new Pair(mSocket, this)); + } + + ((MulticastSocket) mSocket).joinGroup(InetAddress.getByName(address)); } /** @@ -114,13 +128,8 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList * @throws IOException */ public void dropMembership(String address) throws UnknownHostException, IOException { - if (mReceiverTask != null && mReceiverTask.getSocket() instanceof MulticastSocket) { - if (!mReceiverTask.isCancelled()) { - mReceiverTask.cancel(true); - } - - final MulticastSocket socket = (MulticastSocket) mReceiverTask.getSocket(); - socket.leaveGroup(InetAddress.getByName(address)); + if (mSocket instanceof MulticastSocket) { + ((MulticastSocket) mSocket).leaveGroup(InetAddress.getByName(address)); } } @@ -133,10 +142,12 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList * @param callback callback for results * @throws UnknownHostException * @throws IOException + * @throws IllegalStateException if socket is not bound. */ - public void send(String base64String, Integer port, String address, @Nullable Callback callback) throws UnknownHostException, IOException { + public void send(String base64String, Integer port, String address, @Nullable Callback callback) + throws UnknownHostException, IllegalStateException, IOException { if (null == mSocket || !mSocket.isBound()) { - return; + throw new IllegalStateException("Socket is not bound."); } byte[] data = Base64.decode(base64String, Base64.NO_WRAP); @@ -159,11 +170,8 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList * Sets the socket to enable broadcasts. */ public void setBroadcast(boolean flag) throws SocketException { - if (mReceiverTask != null) { - DatagramSocket socket = mReceiverTask.getSocket(); - if (socket != null) { - socket.setBroadcast(flag); - } + if (mSocket != null) { + mSocket.setBroadcast(flag); } } @@ -172,13 +180,11 @@ public final class UdpSocketClient implements UdpReceiverTask.OnDataReceivedList */ public void close() throws IOException { if (mReceiverTask != null && !mReceiverTask.isCancelled()) { - // stop the receiving task, and close the channel + // stop the receiving task mReceiverTask.cancel(true); - if (!mReceiverTask.getSocket().isClosed()) { - mReceiverTask.getSocket().close(); - } } + // close the socket if (mSocket != null && !mSocket.isClosed()) { mSocket.close(); mSocket = null; diff --git a/android/src/main/java/com/tradle/react/UdpSockets.java b/android/src/main/java/com/tradle/react/UdpSockets.java index ec323bf..de355d7 100644 --- a/android/src/main/java/com/tradle/react/UdpSockets.java +++ b/android/src/main/java/com/tradle/react/UdpSockets.java @@ -5,29 +5,29 @@ * Created by Andy Prock on 9/24/15. */ - package com.tradle.react; +package com.tradle.react; - import android.content.Context; - import android.net.wifi.WifiManager; - import android.support.annotation.Nullable; - import android.util.SparseArray; +import android.content.Context; +import android.net.wifi.WifiManager; +import android.support.annotation.Nullable; +import android.util.SparseArray; - import com.facebook.common.logging.FLog; - import com.facebook.react.bridge.Arguments; - import com.facebook.react.bridge.Callback; - import com.facebook.react.bridge.GuardedAsyncTask; - import com.facebook.react.bridge.ReactApplicationContext; - import com.facebook.react.bridge.ReactContext; - import com.facebook.react.bridge.ReactContextBaseJavaModule; - import com.facebook.react.bridge.ReactMethod; - import com.facebook.react.bridge.ReadableMap; - import com.facebook.react.bridge.WritableMap; - import com.facebook.react.modules.core.DeviceEventManagerModule; +import com.facebook.common.logging.FLog; +import com.facebook.react.bridge.Arguments; +import com.facebook.react.bridge.Callback; +import com.facebook.react.bridge.GuardedAsyncTask; +import com.facebook.react.bridge.ReactApplicationContext; +import com.facebook.react.bridge.ReactContext; +import com.facebook.react.bridge.ReactContextBaseJavaModule; +import com.facebook.react.bridge.ReactMethod; +import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.WritableMap; +import com.facebook.react.modules.core.DeviceEventManagerModule; - import java.io.IOException; - import java.net.SocketException; - import java.net.UnknownHostException; - import java.util.concurrent.ExecutionException; +import java.io.IOException; +import java.net.SocketException; +import java.net.UnknownHostException; +import java.util.concurrent.ExecutionException; /** * The NativeModule in charge of storing active {@link UdpSocketClient}s, and acting as an api layer. @@ -185,6 +185,12 @@ public final class UdpSockets extends ReactContextBaseJavaModule try { client.addMembership(multicastAddress); + } catch (IllegalStateException ise) { + // an exception occurred + FLog.e(TAG, "addMembership", ise); + } catch (UnknownHostException uhe) { + // an exception occurred + FLog.e(TAG, "addMembership", uhe); } catch (IOException ioe) { // an exception occurred FLog.e(TAG, "addMembership", ioe); @@ -232,7 +238,9 @@ public final class UdpSockets extends ReactContextBaseJavaModule try { client.send(base64String, port, address, callback); - } catch (UnknownHostException uhe) { + } catch (IllegalStateException ise) { + callback.invoke(UdpErrorUtil.getError(null, ise.getMessage())); + }catch (UnknownHostException uhe) { callback.invoke(UdpErrorUtil.getError(null, uhe.getMessage())); } catch (IOException ioe) { // an exception occurred