Merge pull request #50 from cjmang/thread-safety

Thread safety

Author: cjmang

src/main/java/dev/koifysh/archipelago/Client.java

@@ -1,9 +1,11 @@
 package dev.koifysh.archipelago;
 
+import dev.koifysh.archipelago.bounce.BouncedManager;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 import dev.koifysh.archipelago.events.RetrievedEvent;
 import dev.koifysh.archipelago.flags.ItemsHandling;
+import dev.koifysh.archipelago.bounce.DeathLinkHandler;
 import dev.koifysh.archipelago.network.server.ConnectUpdatePacket;
 import dev.koifysh.archipelago.network.server.RoomInfoPacket;
 import dev.koifysh.archipelago.parts.DataPackage;
@@ -19,19 +21,22 @@
 import java.io.*;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.nio.CharBuffer;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 public abstract class Client {
 
     private final static Logger LOGGER = Logger.getLogger(Client.class.getName());
 
+    public static final Version protocolVersion = new Version(0, 6, 1);
+    private final static Gson gson = new Gson();
+
     private static final String OS = System.getProperty("os.name").toLowerCase();
 
     private static final Path cachePath;
@@ -69,9 +74,7 @@ else if(xdg == null || xdg.isEmpty() )
 
     protected Map<String,String> versions;
 
-    protected ArrayList<String> games;
-
-    private final static Gson gson = new Gson();
+    protected List<String> games;
 
     private int hintPoints;
 
@@ -81,31 +84,33 @@ else if(xdg == null || xdg.isEmpty() )
 
     private RoomInfoPacket roomInfo;
 
-    private final DataPackage dataPackage;
+    private final DataPackage dataPackage = new DataPackage();
 
     public static Client client;
 
     private final LocationManager locationManager;
     private final ItemManager itemManager;
     private final EventManager eventManager;
-
-    public static final Version protocolVersion = new Version(0, 6, 1);
+    private final BouncedManager bouncedManager;
+    private final DeathLinkHandler deathLinkHandler;
 
     private int team;
     private int slot;
-    private HashMap<Integer, NetworkSlot> slotInfo;
+    private Map<Integer, NetworkSlot> slotInfo;
     private String name = "Name not set";
     private String game = "Game not set";
     private String alias;
-    private Set<String> tags = new HashSet<>();
+    private final Set<String> tags = Collections.newSetFromMap(new ConcurrentHashMap<>());
     private int itemsHandlingFlags = 0b000;
 
     public Client() {
         dataPackageLocation = datapackageCachePath;
-        dataPackage = new DataPackage();
         eventManager = new EventManager();
         locationManager = new LocationManager(this);
         itemManager = new ItemManager(this);
+        bouncedManager = new BouncedManager();
+        deathLinkHandler = new DeathLinkHandler(this);
+        bouncedManager.addHandler(deathLinkHandler);
         client = this;
     }
 
@@ -124,7 +129,8 @@ public void setGame(String game) {
      */
     public void setTags(Set<String> tags) {
         if (!this.tags.equals(tags)) {
-            this.tags = tags;
+            this.tags.clear();
+            this.tags.addAll(tags);
             if (isConnected()) {
                 ConnectUpdatePacket packet = new ConnectUpdatePacket();
                 packet.tags = this.tags;
@@ -138,8 +144,7 @@ public void setTags(Set<String> tags) {
      * @param tag String tag to be added.
      */
     public void addTag(String tag) {
-        if (!this.tags.contains(tag)) {
-            tags.add(tag);
+        if(tags.add(tag)) {
             if (isConnected()) {
                 ConnectUpdatePacket packet = new ConnectUpdatePacket();
                 packet.tags = this.tags;
@@ -153,8 +158,7 @@ public void addTag(String tag) {
      * @param tag String tag to be removed.
      */
     public void removeTag(String tag) {
-        if (this.tags.contains(tag)) {
-            tags.remove(tag);
+        if(tags.remove(tag)) {
             if (isConnected()) {
                 ConnectUpdatePacket packet = new ConnectUpdatePacket();
                 packet.tags = this.tags;
@@ -352,7 +356,7 @@ void setTeam(int team) {
         this.team = team;
     }
 
-    void setSlotInfo(HashMap<Integer, NetworkSlot> slotInfo) {
+    void setSlotInfo(Map<Integer, NetworkSlot> slotInfo) {
         this.slotInfo = slotInfo;
     }
 
@@ -388,7 +392,7 @@ public RoomInfoPacket getRoomInfo() {
         return roomInfo;
     }
 
-    public HashMap<Integer, NetworkSlot> getSlotInfo() {return slotInfo;}
+    public Map<Integer, NetworkSlot> getSlotInfo() {return slotInfo;}
 
     /**
      * Works exactly like {@link #connect(URI, boolean)} with allowDowngrade set to true;
@@ -629,6 +633,14 @@ public EventManager getEventManager() {
         return eventManager;
     }
 
+    /**
+     * @return the bounced packet handler
+     */
+    public BouncedManager getBouncedManager()
+    {
+        return bouncedManager;
+    }
+
     /**
      * Uses DataStorage to save a value on the AP server.
      *
@@ -705,4 +717,26 @@ public int dataStorageGet(Collection<String> keys) {
         return getPacket.getRequestID();
     }
 
+    /**
+     * Helper for sending a death link bounce packet. You can send these without enabling death link first, but it is frowned upon.
+     * @param source A String that is the name of the player sending the death link (does not have to be slot name)
+     * @param cause A String that is the cause of this death. may be empty.
+     */
+    public void sendDeathlink(String source, String cause)
+    {
+        deathLinkHandler.sendDeathLink(source, cause);
+    }
+
+    /**
+     * Enable or disable receiving death links.
+     * @param enabled set to TRUE to enable death links, FALSE to disable.
+     */
+    public void setDeathLinkEnabled(boolean enabled) {
+        if(enabled)
+            addTag(DeathLinkHandler.DEATHLINK_TAG);
+        else
+            removeTag(DeathLinkHandler.DEATHLINK_TAG);
+    }
+
+
 }

src/main/java/dev/koifysh/archipelago/ItemManager.java

@@ -6,38 +6,45 @@
 import dev.koifysh.archipelago.parts.NetworkItem;
 
 import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentLinkedDeque;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicInteger;
 
 public class ItemManager {
 
 
-    Client client;
-    WebSocket webSocket;
+    private final Client client;
+    private WebSocket webSocket;
 
-    ArrayList<NetworkItem> receivedItems = new ArrayList<>();
+    private List<NetworkItem> receivedItems = new ArrayList<>();
 
-    int index;
+    private final AtomicInteger index = new AtomicInteger();
 
     public ItemManager(Client client) {
         this.client = client;
     }
 
-    public void receiveItems(ArrayList<NetworkItem> ids, int index) {
+    public void receiveItems(List<NetworkItem> ids, int index) {
         if (index == 0) {
             receivedItems = new ArrayList<>();
         }
         if (receivedItems.size() == index) {
-            receivedItems.addAll(ids);
+            synchronized (this) {
+                receivedItems.addAll(ids);
+            }
             DataPackage dp = client.getDataPackage();
             int myTeam = client.getTeam();
-            for (int i = this.index; i < receivedItems.size(); i++) {
+            for (int i = this.index.get(); i < receivedItems.size(); i++) {
                 NetworkItem item = receivedItems.get(i);
                 item.itemName = dp.getItem(item.itemID, client.getGame());
                 item.locationName = dp.getLocation(item.locationID, client.getSlotInfo().get(item.playerID).game);
                 item.playerName = client.getRoomInfo().getPlayer(myTeam,item.playerID).alias;
                 client.getEventManager().callEvent(new ReceiveItemEvent(item, i+1));
             }
 
-            this.index = receivedItems.size();
+            this.index.set(receivedItems.size());
         }
         else {
             if(webSocket != null) {
@@ -47,27 +54,31 @@ public void receiveItems(ArrayList<NetworkItem> ids, int index) {
         }
     }
 
-    public void writeFromSave(ArrayList<NetworkItem> receivedItems, int index) {
-        this.receivedItems = receivedItems;
-        this.index = index;
+    public void writeFromSave(List<NetworkItem> receivedItems, int index) {
+        this.receivedItems = new ArrayList<>(receivedItems);
+        this.index.set(index);
     }
 
-    public void setAPWebSocket(WebSocket webSocket) {
+    void setAPWebSocket(WebSocket webSocket) {
         this.webSocket = webSocket;
     }
 
     public int getIndex() {
-        return index;
+        return index.get();
     }
 
-    public ArrayList<NetworkItem> getReceivedItems() {
-        return receivedItems;
+    public List<NetworkItem> getReceivedItems() {
+        synchronized (this) {
+            return new ArrayList<>(receivedItems);
+        }
     }
 
-    public ArrayList<Long> getReceivedItemIDs() {
-        ArrayList<Long> ids = new ArrayList<>();
-        for (NetworkItem receivedItem : receivedItems) {
-            ids.add(receivedItem.itemID);
+    public List<Long> getReceivedItemIDs() {
+        List<Long> ids = new ArrayList<>();
+        synchronized (this) {
+            for (NetworkItem receivedItem : receivedItems) {
+                ids.add(receivedItem.itemID);
+            }
         }
         return ids;
     }

src/main/java/dev/koifysh/archipelago/LocationManager.java

@@ -2,19 +2,18 @@
 
 import dev.koifysh.archipelago.network.client.LocationChecks;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class LocationManager {
 
-    Client client;
-    WebSocket webSocket;
+    // TODO: why is this field unused?
+    private final Client client;
+    private WebSocket webSocket;
 
-    Set<Long> checkedLocations = new HashSet<>();
+    private final Set<Long> checkedLocations = Collections.newSetFromMap(new ConcurrentHashMap<>());
 
-    Set<Long> missingLocations = new HashSet<>();
+    private final Set<Long> missingLocations = Collections.newSetFromMap(new ConcurrentHashMap<>());
 
     public LocationManager(Client client) {
         this.client = client;
@@ -60,7 +59,7 @@ public void resendAllCheckedLocations() {
         webSocket.sendPacket(packet);
     }
 
-    protected void setAPWebSocket(WebSocket webSocket) {
+    void setAPWebSocket(WebSocket webSocket) {
         this.webSocket = webSocket;
     }
 
@@ -77,7 +76,8 @@ public void addCheckedLocations(Set<Long> newLocations) {
         this.missingLocations.removeAll(newLocations);
     }
 
-    public void setMissingLocations(HashSet<Long> missingLocations) {
-        this.missingLocations = missingLocations;
+    public void setMissingLocations(Set<Long> missingLocations) {
+        this.missingLocations.clear();
+        this.missingLocations.addAll(missingLocations);
     }
 }

src/main/java/dev/koifysh/archipelago/WebSocket.java

@@ -9,7 +9,6 @@
 import dev.koifysh.archipelago.Print.APPrintPart;
 import dev.koifysh.archipelago.Print.APPrintType;
 import dev.koifysh.archipelago.flags.NetworkPlayer;
-import dev.koifysh.archipelago.helper.DeathLink;
 import dev.koifysh.archipelago.network.APPacket;
 import dev.koifysh.archipelago.network.ConnectionResult;
 import dev.koifysh.archipelago.network.client.*;
@@ -200,10 +199,10 @@ else if (part.type == APPrintType.locationID) {
                         break;
                     case Bounced:
                         BouncedPacket bounced = gson.fromJson(packet, BouncedPacket.class);
-                        if (bounced.tags.contains("DeathLink"))
-                            DeathLink.receiveDeathLink(bounced);
-                        else
+                        if(!client.getBouncedManager().handle(bounced))
+                        {
                             client.getEventManager().callEvent(new BouncedEvent(bounced.games, bounced.tags, bounced.slots, bounced.data));
+                        }
                         break;
                     case LocationInfo:
                         LocationInfoPacket locations = gson.fromJson(packet, LocationInfoPacket.class);