diff --git a/ts3-client/core/src/main/java/com/ts3client/net/ChannelNode.java b/ts3-client/core/src/main/java/com/ts3client/net/ChannelNode.java index 6ec902d..211b3b8 100644 --- a/ts3-client/core/src/main/java/com/ts3client/net/ChannelNode.java +++ b/ts3-client/core/src/main/java/com/ts3client/net/ChannelNode.java @@ -3,26 +3,30 @@ package com.ts3client.net; import java.util.ArrayList; import java.util.List; -/** Mutable view-model of a TeamSpeak channel. */ +/** + * View-model of a TeamSpeak channel. The event thread updates it while the UI reads it, so + * every field is volatile; {@link #children} and {@link #clients} are replaced whole on each + * {@link ServerModel#buildTree()}, never changed in place. + */ public final class ChannelNode { public final int id; - public int parentId; - public int order; - public String name; - public String topic = ""; - public String description = ""; - public boolean descriptionLoaded; - public boolean hasPassword; - public boolean permanent; - public int maxClients = -1; + public volatile int parentId; + public volatile int order; + public volatile String name; + public volatile String topic = ""; + public volatile String description = ""; + public volatile boolean descriptionLoaded; + public volatile boolean hasPassword; + public volatile boolean permanent; + public volatile int maxClients = -1; /** Id of the channel's custom icon in the server's file repository, or 0 for none. */ - public long iconId; + public volatile long iconId; /** Whether the server sends us this channel's client list; set from the subscription events. */ - public boolean subscribed; + public volatile boolean subscribed; - /** Populated when the tree is rebuilt. */ - public final List children = new ArrayList<>(); - public final List clients = new ArrayList<>(); + /** As of the last {@link ServerModel#buildTree()}; unmodifiable. */ + public volatile List children = List.of(); + public volatile List clients = List.of(); public ChannelNode(int id, String name) { this.id = id; diff --git a/ts3-client/core/src/main/java/com/ts3client/net/ClientEntry.java b/ts3-client/core/src/main/java/com/ts3client/net/ClientEntry.java index d0f385f..20502ac 100644 --- a/ts3-client/core/src/main/java/com/ts3client/net/ClientEntry.java +++ b/ts3-client/core/src/main/java/com/ts3client/net/ClientEntry.java @@ -1,38 +1,41 @@ package com.ts3client.net; -/** Mutable view-model of a connected client. */ +/** + * View-model of a connected client. The event and audio threads update it while the UI reads + * it, so every field is volatile; {@link #serverGroupIds} is replaced, never changed in place. + */ public final class ClientEntry { public final int id; - public int channelId; - public String nickname; - public String uniqueId = ""; - public int databaseId; - public int type; // 0 = normal voice client, 1 = server-query - public int talkPower; + public volatile int channelId; + public volatile String nickname; + public volatile String uniqueId = ""; + public volatile int databaseId; + public volatile int type; // 0 = normal voice client, 1 = server-query + public volatile int talkPower; - public int[] serverGroupIds = new int[0]; - public int channelGroupId; + public volatile int[] serverGroupIds = new int[0]; + public volatile int channelGroupId; // Filled on demand from clientinfo. - public String platform = ""; - public String version = ""; - public long idleTimeMs; - public String description = ""; + public volatile String platform = ""; + public volatile String version = ""; + public volatile long idleTimeMs; + public volatile String description = ""; /** MD5 of the client's avatar ({@code client_flag_avatar}); empty when they have none. */ - public String avatarFlag = ""; + public volatile String avatarFlag = ""; - public boolean talking; - public boolean inputMuted; // microphone muted (client_input_muted) - public boolean outputMuted; // speakers muted / deafened (client_output_muted) + public volatile boolean talking; + public volatile boolean inputMuted; // microphone muted (client_input_muted) + public volatile boolean outputMuted; // speakers muted / deafened (client_output_muted) /** False while the capture device is unavailable: another tab holds the microphone. */ - public boolean inputHardware = true; + public volatile boolean inputHardware = true; /** False while the playback device is unavailable. */ - public boolean outputHardware = true; - public boolean away; + public volatile boolean outputHardware = true; + public volatile boolean away; /** The message published with the away state, empty when there is none. */ - public String awayMessage = ""; - public boolean channelCommander; - public boolean self; + public volatile String awayMessage = ""; + public volatile boolean channelCommander; + public volatile boolean self; public ClientEntry(int id, String nickname) { this.id = id; @@ -50,25 +53,27 @@ public final class ClientEntry { /** Adds a server group id, if not already present. */ public void addServerGroup(int groupId) { - for (int id : serverGroupIds) if (id == groupId) return; - int[] updated = java.util.Arrays.copyOf(serverGroupIds, serverGroupIds.length + 1); - updated[serverGroupIds.length] = groupId; + int[] ids = serverGroupIds; + for (int id : ids) if (id == groupId) return; + int[] updated = java.util.Arrays.copyOf(ids, ids.length + 1); + updated[ids.length] = groupId; serverGroupIds = updated; } /** Removes a server group id, if present. */ public void removeServerGroup(int groupId) { + int[] ids = serverGroupIds; int index = -1; - for (int i = 0; i < serverGroupIds.length; i++) { - if (serverGroupIds[i] == groupId) { + for (int i = 0; i < ids.length; i++) { + if (ids[i] == groupId) { index = i; break; } } if (index < 0) return; - int[] updated = new int[serverGroupIds.length - 1]; - System.arraycopy(serverGroupIds, 0, updated, 0, index); - System.arraycopy(serverGroupIds, index + 1, updated, index, serverGroupIds.length - index - 1); + int[] updated = new int[ids.length - 1]; + System.arraycopy(ids, 0, updated, 0, index); + System.arraycopy(ids, index + 1, updated, index, ids.length - index - 1); serverGroupIds = updated; } } diff --git a/ts3-client/core/src/main/java/com/ts3client/net/ServerModel.java b/ts3-client/core/src/main/java/com/ts3client/net/ServerModel.java index e56f2da..50834ac 100644 --- a/ts3-client/core/src/main/java/com/ts3client/net/ServerModel.java +++ b/ts3-client/core/src/main/java/com/ts3client/net/ServerModel.java @@ -2,15 +2,16 @@ package com.ts3client.net; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; /** - * Thread-safe holder for the current server state (channels + clients). - * - *

Mutated from ts3j's event thread and read from the Swing EDT while building - * the tree, so all access is synchronised on the instance. + * The current server state (channels + clients), updated from ts3j's event thread and read + * from the UI. The maps are guarded by the instance; the {@link ChannelNode}s and + * {@link ClientEntry}s handed out are live, with volatile fields, so a reader sees each value + * as it is now but not several of them changed together. */ public final class ServerModel { @@ -198,7 +199,12 @@ public final class ServerModel { public synchronized boolean canJoinChannel(int channelId) { ChannelNode channel = channels.get(channelId); if (channel == null || !channel.subscribed) return false; - return channel.maxClients < 0 || channel.clients.size() < channel.maxClients; + if (channel.maxClients < 0) return true; + int inside = 0; + for (ClientEntry c : clients.values()) { + if (c.channelId == channelId && !c.isQuery()) inside++; + } + return inside < channel.maxClients; } public synchronized String getServerName() { @@ -380,24 +386,23 @@ public final class ServerModel { * @return the list of root channels (parentId == 0) */ public synchronized List buildTree() { - // Reset transient child/client lists. - for (ChannelNode c : channels.values()) { - c.children.clear(); - c.clients.clear(); - } + // Fresh lists, published whole: whoever still walks the previous tree keeps a consistent one. + Map> children = new HashMap<>(); + Map> inside = new HashMap<>(); List roots = new ArrayList<>(); for (ChannelNode c : channels.values()) { ChannelNode parent = channels.get(c.parentId); if (c.parentId == 0 || parent == null) { roots.add(c); } else { - parent.children.add(c); + children.computeIfAbsent(parent.id, id -> new ArrayList<>()).add(c); } } for (ClientEntry cl : clients.values()) { if (cl.isQuery()) continue; // hide server-query clients from the tree - ChannelNode ch = channels.get(cl.channelId); - if (ch != null) ch.clients.add(cl); + if (channels.containsKey(cl.channelId)) { + inside.computeIfAbsent(cl.channelId, id -> new ArrayList<>()).add(cl); + } } Comparator byClient = Comparator @@ -406,8 +411,12 @@ public final class ServerModel { sortSiblings(roots); for (ChannelNode c : channels.values()) { - sortSiblings(c.children); - c.clients.sort(byClient); + List below = children.getOrDefault(c.id, List.of()); + sortSiblings(below); + c.children = List.copyOf(below); + List members = inside.getOrDefault(c.id, List.of()); + if (members.size() > 1) members.sort(byClient); + c.clients = List.copyOf(members); } return roots; } diff --git a/ts3-client/core/src/test/java/com/ts3client/net/ServerModelRulesTest.java b/ts3-client/core/src/test/java/com/ts3client/net/ServerModelRulesTest.java index 255fa27..ca483e9 100644 --- a/ts3-client/core/src/test/java/com/ts3client/net/ServerModelRulesTest.java +++ b/ts3-client/core/src/test/java/com/ts3client/net/ServerModelRulesTest.java @@ -53,7 +53,6 @@ class ServerModelRulesTest { channel.subscribed = true; assertTrue(model.canJoinChannel(1)); channel.maxClients = 1; - channel.clients.add(model.getClient(5)); assertFalse(model.canJoinChannel(1), "full"); assertFalse(model.canJoinChannel(2), "unknown"); }