From 3b3fddeeec80e2c111ddffa72d3106034711d12b Mon Sep 17 00:00:00 2001 From: Lipi Lee Date: Tue, 20 Jun 2017 15:45:10 +0900 Subject: [PATCH 1/3] Remove a duplicated INTERNET permission as the build have gotten the below warning DataCollectorsClients/VpnCollector/app/src/main/AndroidManifest.xml:23:5-67 Warning: Element uses-permission#android.permission.INTERNET at AndroidManifest.xml:23:5-67 duplicated with element declared at AndroidManifest.xml:11:5-67 --- .../VpnCollector/app/src/main/AndroidManifest.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/DataCollectorsClients/VpnCollector/app/src/main/AndroidManifest.xml b/DataCollectorsClients/VpnCollector/app/src/main/AndroidManifest.xml index 432d2958c..95b15ff82 100755 --- a/DataCollectorsClients/VpnCollector/app/src/main/AndroidManifest.xml +++ b/DataCollectorsClients/VpnCollector/app/src/main/AndroidManifest.xml @@ -8,7 +8,6 @@ android:minSdkVersion="19" android:targetSdkVersion="21" /> - @@ -113,4 +112,4 @@ - \ No newline at end of file + From 47ada6def6d375b0c59ffa02590762aeb7912b93 Mon Sep 17 00:00:00 2001 From: Lipi Lee Date: Tue, 20 Jun 2017 15:59:18 +0900 Subject: [PATCH 2/3] Simplify SessionTable with ConcurrentHashMap for thread safety --- .../com/att/arotcpcollector/SessionTable.java | 84 +++---------------- 1 file changed, 12 insertions(+), 72 deletions(-) diff --git a/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionTable.java b/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionTable.java index 0515c2b50..c7e6f9eb9 100755 --- a/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionTable.java +++ b/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionTable.java @@ -18,106 +18,46 @@ import android.support.annotation.NonNull; import android.util.Log; -import java.util.Collection; -import java.util.Hashtable; -import java.util.Map; -import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** * Created by Bharath. */ - -class SessionTable implements Map{ +class SessionTable extends ConcurrentHashMap { private static final String TAG = "SessionTable"; private final int limit; - private final Hashtable sessionTable = new Hashtable<>(); - public SessionTable(int limit) { + SessionTable(int limit) { this.limit = limit; } @Override - public void clear() { - sessionTable.clear(); - } - - @Override - public boolean containsKey(Object key) { - return sessionTable.containsKey(key); - } - - @Override - public boolean containsValue(Object value) { - return sessionTable.containsValue(value); - } - - @NonNull - @Override - public Set> entrySet() { - return sessionTable.entrySet(); - } - - @Override - public Session get(Object key) { - return sessionTable.get(key); - } - - @Override - public boolean isEmpty() { - return sessionTable.isEmpty(); - } - - @NonNull - @Override - public Set keySet() { - return sessionTable.keySet(); - } - - @Override - public synchronized Session put(String key, Session value) { - if(sessionTable.size() >= limit) { + public Session put(@NonNull String key, @NonNull Session value) { + if(size() >= limit) { evictEntry(); } - return sessionTable.put(key, value); + + return super.put(key, value); } private void evictEntry() { long oldest = Long.MAX_VALUE; Session oldSession = null; - for(Entry se : sessionTable.entrySet()) { - Session session = se.getValue(); + + for(final Session session: values()) { if(oldest > session.getLastAccessed()) { oldest = session.getLastAccessed(); oldSession = session; } } + if(oldSession != null) { - Log.i(TAG, "Eviction initiated:" + oldSession.getSessionKey()); + Log.i(TAG, "Eviction initiated: " + oldSession.getSessionKey()); SessionManager.getInstance().closeSession(oldSession); } else { Log.e(TAG, "Error evicting entry"); } } - - @Override - public void putAll(@NonNull Map map) { - throw new UnsupportedOperationException("This operation is not supported"); - } - - @Override - public Session remove(Object key) { - return sessionTable.remove(key); - } - - @Override - public int size() { - return sessionTable.size(); - } - - @NonNull - @Override - public Collection values() { - return sessionTable.values(); - } } + From 1d32c442b2cb527a582051f7eed12cee1931c23c Mon Sep 17 00:00:00 2001 From: Lipi Lee Date: Fri, 23 Jun 2017 15:28:26 +0900 Subject: [PATCH 3/3] Remove the unnecessary synchronized keywords Delete synchronized when handling a modified SessionTable and change some for loops to foreachs --- .../att/arotcpcollector/SessionManager.java | 151 ++++++------------ 1 file changed, 48 insertions(+), 103 deletions(-) diff --git a/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionManager.java b/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionManager.java index f6ab95b5f..abc95a820 100755 --- a/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionManager.java +++ b/DataCollectorsClients/VpnCollector/app/src/main/java/com/att/arotcpcollector/SessionManager.java @@ -16,6 +16,7 @@ package com.att.arotcpcollector; +import android.support.annotation.Nullable; import android.util.Log; import com.att.arotcpcollector.ip.IPv4Header; @@ -36,7 +37,6 @@ import java.nio.channels.SocketChannel; import java.nio.channels.UnresolvedAddressException; import java.nio.channels.UnsupportedAddressTypeException; -import java.util.Hashtable; import java.util.Iterator; /** @@ -50,7 +50,6 @@ public class SessionManager { private static Object syncObj = new Object(); private static volatile SessionManager instance = null; private SessionTable table = null; - public static Object syncTable = new Object(); private SocketProtector protector = null; Selector selector; @@ -90,9 +89,7 @@ public void keepSessionAlive(Session session) { if (sessionKey == null) { sessionKey = this.createKey(session.getDestAddress(), session.getDestPort(), session.getSourceIp(), session.getSourcePort()); } - synchronized (syncTable) { - table.put(sessionKey, session); - } + table.put(sessionKey, session); } } @@ -163,82 +160,52 @@ public Session getSession(int ipAddress, int destPort, int srcIpAddress, int src String sessionKey = createKey(ipAddress, destPort, srcIpAddress, srcPort); Session session = null; - synchronized (syncTable) { - if (table.containsKey(sessionKey)) { - session = table.get(sessionKey); - } + if (table.containsKey(sessionKey)) { + session = table.get(sessionKey); } return session; } public Session getSessionByKey(String sessionKey) { Session session = null; - synchronized (syncTable) { - if (table.containsKey(sessionKey)) { - session = table.get(sessionKey); - } + + if (table.containsKey(sessionKey)) { + session = table.get(sessionKey); } return session; } + @Nullable public Session getSessionByDatagramChannel(DatagramChannel channel) { - Session session = null; - synchronized (syncTable) { - Iterator it = table.values().iterator(); - while (it.hasNext()) { - Session sess = it.next(); - if (sess.getUdpChannel() == channel) { - session = sess; - break; - } + for (final Session session: table.values()) { + if (session.getUdpChannel() == channel) { + return session; } } - return session; + return null; } + @Nullable public Session getSessionByChannel(SocketChannel channel) { - Session session = null; - synchronized (syncTable) { - Iterator it = table.values().iterator(); - while (it.hasNext()) { - Session sess = it.next(); - if (sess.getSocketchannel() == channel) { - session = sess; - break; - } + for (final Session session: table.values()) { + if (session.getSocketchannel() == channel) { + return session; } } - return session; + return null; } public void removeSessionByChannel(SocketChannel channel) { - String key = null; - String tmp = null; - Session session = null; - synchronized (syncTable) { - Iterator it = table.keySet().iterator(); - while (it.hasNext()) { - tmp = it.next(); - Session sess = table.get(tmp); - if (sess != null && sess.getSocketchannel() == channel) { - key = tmp; - session = sess; - break; - } - - } - } - if (key != null) { - synchronized (syncTable) { + for (final String key: table.keySet()) { + Session session = table.get(key); + if (session != null && session.getSocketchannel() == channel) { table.remove(key); + Log.d(TAG, + "closed session -> " + PacketUtil.intToIPAddress(session.getDestAddress()) + ":" + session.getDestPort() + "-" + + PacketUtil.intToIPAddress(session.getSourceIp()) + ":" + session.getSourcePort()); + break; } } - if (session != null) { - Log.d(TAG, - "closed session -> " + PacketUtil.intToIPAddress(session.getDestAddress()) + ":" + session.getDestPort() + "-" - + PacketUtil.intToIPAddress(session.getSourceIp()) + ":" + session.getSourcePort()); - session = null; - } } /** @@ -251,10 +218,8 @@ public void removeSessionByChannel(SocketChannel channel) { */ public void closeSession(int ip, int port, int srcIp, int srcPort) { String keys = createKey(ip, port, srcIp, srcPort); - Session session = null; //getSession(ip, port, srcIp, srcPort); - synchronized (syncTable) { - session = table.remove(keys); - } + Session session = table.remove(keys); //getSession(ip, port, srcIp, srcPort); + if (session != null) { try { SocketChannel chan = session.getSocketchannel(); @@ -267,7 +232,6 @@ public void closeSession(int ip, int port, int srcIp, int srcPort) { Log.d(TAG, "closed session -> " + PacketUtil.intToIPAddress(session.getDestAddress()) + ":" + session.getDestPort() + "-" + PacketUtil.intToIPAddress(session.getSourceIp()) + ":" + session.getSourcePort()); - session = null; } } @@ -286,23 +250,19 @@ public void closeSession(Session session) { sessionKey = this.createKey(session.getDestAddress(), session.getDestPort(), session.getSourceIp(), session.getSourcePort()); } - synchronized (syncTable) { - table.remove(sessionKey); - } - if (session != null) { - try { - SocketChannel chan = session.getSocketchannel(); - if (chan != null) { - chan.close(); - } - } catch (IOException e) { - e.printStackTrace(); + table.remove(sessionKey); + + try { + SocketChannel chan = session.getSocketchannel(); + if (chan != null) { + chan.close(); } - Log.d(TAG, - "closed session -> " + PacketUtil.intToIPAddress(session.getDestAddress()) + ":" + session.getDestPort() + "-" - + PacketUtil.intToIPAddress(session.getSourceIp()) + ":" + session.getSourcePort()); - session = null; + } catch (IOException e) { + e.printStackTrace(); } + Log.d(TAG, + "closed session -> " + PacketUtil.intToIPAddress(session.getDestAddress()) + ":" + session.getDestPort() + "-" + + PacketUtil.intToIPAddress(session.getSourceIp()) + ":" + session.getSourcePort()); } /** @@ -316,11 +276,8 @@ public void closeSession(Session session) { */ public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) { String sessionKey = createKey(ip, port, srcIp, srcPort); - boolean found = false; - synchronized (syncTable) { - found = table.containsKey(sessionKey); - } - if (found) { + + if (table.containsKey(sessionKey)) { return null; } Session ses = new Session(); @@ -386,20 +343,16 @@ public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) { ses.setUdpChannel(channel); - synchronized (syncTable) { - if (!table.containsKey(sessionKey)) { - table.put(sessionKey, ses); - } else { - found = true; - } - } - if (found) { + if (table.containsKey(sessionKey)) { try { channel.close(); } catch (IOException e) { e.printStackTrace(); } + } else { + table.put(sessionKey, ses); } + if (ses != null) { Log.d(TAG, "new UDP session successfully created."); } @@ -418,11 +371,8 @@ public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) { */ public Session createNewSession(int ip, int port, int srcIp, int srcPort) throws SessionCreateException { String sessionKey = createKey(ip, port, srcIp, srcPort); - boolean found = false; - synchronized (syncTable) { - found = table.containsKey(sessionKey); - } - if (found) { + + if (table.containsKey(sessionKey)) { Session session = table.get(sessionKey); session.setAbortingConnection(true); throw new SessionCreateException("Session already exist"); @@ -496,20 +446,15 @@ public Session createNewSession(int ip, int port, int srcIp, int srcPort) throws session.setSocketchannel(channel); - synchronized (syncTable) { - if (!table.containsKey(sessionKey)) { - table.put(sessionKey, session); - } else { - found = true; - } - } - if (found) { + if (table.containsKey(sessionKey)) { try { channel.close(); } catch (IOException e) { e.printStackTrace(); } session = null; + } else { + table.put(sessionKey, session); } return session;