Skip to content

Commit d6ba33a

Browse files
author
games647
committed
Fix ConcurrentModification for TopList (Fixes #215)
1 parent 9cb2f54 commit d6ba33a

File tree

8 files changed

+92
-75
lines changed

8 files changed

+92
-75
lines changed

‎plugin/src/main/java/com/github/games647/scoreboardstats/scoreboard/PacketListener.java‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import java.util.Collection;
1111
import java.util.Optional;
12+
import java.util.UUID;
1213

1314
import org.bukkit.Bukkit;
1415
import org.bukkit.entity.Player;
@@ -49,21 +50,33 @@ public void onPacketSending(PacketEvent packetEvent) {
4950
return;
5051
}
5152

52-
PacketType packetType = packetEvent.getPacketType();
53-
54-
//everything was read from the packet, so we don't need to access it anymore
55-
//we could now run a sync thread to synchronize with async packets
56-
Bukkit.getScheduler().runTask(plugin, () -> {
57-
if (packetType.equals(SCOREBOARD_SCORE)) {
58-
handleScorePacket(player, packet);
59-
} else if (packetType.equals(SCOREBOARD_OBJECTIVE)) {
60-
handleObjectivePacket(player, packet);
61-
} else if (packetType.equals(SCOREBOARD_DISPLAY_OBJECTIVE)) {
62-
handleDisplayPacket(player, packet);
63-
} else if (packetType.equals(SCOREBOARD_TEAM)) {
64-
handleTeamPacket(player, packet);
65-
}
66-
});
53+
UUID playerUUID = player.getUniqueId();
54+
55+
//handle async packets by other plugins
56+
if (Bukkit.isPrimaryThread()) {
57+
ensureMainThread(playerUUID, packet);
58+
} else {
59+
PacketContainer clone = packet.deepClone();
60+
Bukkit.getScheduler().runTask(plugin, () -> ensureMainThread(playerUUID, clone));
61+
}
62+
}
63+
64+
private void ensureMainThread(UUID uuid, PacketContainer packet) {
65+
Player player = Bukkit.getPlayer(uuid);
66+
if (player == null) {
67+
return;
68+
}
69+
70+
PacketType packetType = packet.getType();
71+
if (packetType.equals(SCOREBOARD_SCORE)) {
72+
handleScorePacket(player, packet);
73+
} else if (packetType.equals(SCOREBOARD_OBJECTIVE)) {
74+
handleObjectivePacket(player, packet);
75+
} else if (packetType.equals(SCOREBOARD_DISPLAY_OBJECTIVE)) {
76+
handleDisplayPacket(player, packet);
77+
} else if (packetType.equals(SCOREBOARD_TEAM)) {
78+
handleTeamPacket(player, packet);
79+
}
6780
}
6881

6982
private void handleScorePacket(Player player, PacketContainer packet) {

‎plugin/src/main/java/com/github/games647/scoreboardstats/scoreboard/PacketManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public PacketManager(ScoreboardStats plugin) {
3838

3939
public PlayerScoreboard getScoreboard(Player player) {
4040
return scoreboards
41-
.computeIfAbsent(player.getUniqueId(), key -> new PlayerScoreboard(player));
41+
.computeIfAbsent(player.getUniqueId(), key -> new PlayerScoreboard(plugin, player));
4242
}
4343

4444
@Override

‎plugin/src/main/java/com/github/games647/scoreboardstats/scoreboard/PlayerScoreboard.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,22 @@
1111
import java.util.Optional;
1212

1313
import org.bukkit.entity.Player;
14-
import org.bukkit.plugin.java.JavaPlugin;
1514

1615
/**
1716
* Represents the scoreboard overview in a server-side perspective.
1817
*/
1918
public class PlayerScoreboard {
2019

20+
private final ScoreboardStats plugin;
21+
private final Player player;
22+
2123
final Map<String, Objective> objectivesByName = Maps.newHashMap();
2224
final Map<String, Team> teamsByName = Maps.newHashMap();
2325

24-
private final Player player;
2526
Objective sidebarObjective;
2627

27-
public PlayerScoreboard(Player player) {
28+
public PlayerScoreboard(ScoreboardStats plugin, Player player) {
29+
this.plugin = plugin;
2830
this.player = player;
2931
}
3032

@@ -101,7 +103,7 @@ void sendPacket(PacketContainer packet) {
101103
ProtocolLibrary.getProtocolManager().sendServerPacket(player, packet);
102104
} catch (InvocationTargetException ex) {
103105
//just log it for now.
104-
JavaPlugin.getPlugin(ScoreboardStats.class).getLog().info("Failed to send packet", ex);
106+
plugin.getLog().info("Failed to send packet", ex);
105107
}
106108
}
107109
}

‎pvp/src/main/java/com/github/games647/scoreboardstats/pvp/Database.java‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.github.games647.scoreboardstats.config.Settings;
44
import com.github.games647.scoreboardstats.variables.ReplaceManager;
55
import com.github.games647.scoreboardstats.variables.Replacer;
6+
import com.google.common.collect.ImmutableMap;
67
import com.google.common.collect.Lists;
78
import com.google.common.collect.Maps;
89
import com.zaxxer.hikari.HikariDataSource;
@@ -12,7 +13,6 @@
1213
import java.sql.ResultSet;
1314
import java.sql.SQLException;
1415
import java.sql.Statement;
15-
import java.sql.Timestamp;
1616
import java.time.Instant;
1717
import java.util.Collection;
1818
import java.util.Collections;
@@ -95,13 +95,12 @@ public void loadAccountAsync(Player player) {
9595
* @param uniqueId the associated playername or uuid
9696
* @return the loaded stats
9797
*/
98-
public Optional<PlayerStats> loadAccount(Object uniqueId) {
99-
if (uniqueId == null || dataSource == null) {
98+
public Optional<PlayerStats> loadAccount(UUID uniqueId) {
99+
if (dataSource == null) {
100100
return Optional.empty();
101101
} else {
102102
try (Connection conn = dataSource.getConnection();
103-
PreparedStatement stmt = conn.prepareStatement("SELECT * FROM player_stats WHERE "
104-
+ "uuid=?")) {
103+
PreparedStatement stmt = conn.prepareStatement("SELECT * FROM player_stats WHERE uuid=?")) {
105104

106105
stmt.setString(1, uniqueId.toString());
107106
try (ResultSet resultSet = stmt.executeQuery()) {
@@ -142,11 +141,7 @@ private PlayerStats extractPlayerStats(ResultSet resultSet) throws SQLException
142141
* @return the loaded stats
143142
*/
144143
public Optional<PlayerStats> loadAccount(Player player) {
145-
if (player == null || dataSource == null) {
146-
return Optional.empty();
147-
} else {
148-
return loadAccount(player.getUniqueId());
149-
}
144+
return loadAccount(player.getUniqueId());
150145
}
151146

152147
/**
@@ -185,7 +180,7 @@ private void update(Collection<PlayerStats> stats) {
185180
//Save the stats to the database
186181
try (Connection conn = dataSource.getConnection();
187182
PreparedStatement stmt = conn.prepareStatement("UPDATE player_stats "
188-
+ "SET kills=?, deaths=?, killstreak=?, mobkills=?, last_online=?, playername=? "
183+
+ "SET kills=?, deaths=?, killstreak=?, mobkills=?, last_online=CURRENT_TIMESTAMP, playername=? "
189184
+ "WHERE id=?")) {
190185
conn.setAutoCommit(false);
191186
for (PlayerStats stat : stats) {
@@ -194,10 +189,9 @@ private void update(Collection<PlayerStats> stats) {
194189
stmt.setInt(3, stat.getKillstreak());
195190
stmt.setInt(4, stat.getMobkills());
196191

197-
stmt.setTimestamp(5, Timestamp.from(stat.getLastOnlineDate()));
198-
stmt.setString(6, stat.getPlayername());
192+
stmt.setString(5, stat.getPlayername());
199193

200-
stmt.setInt(7, stat.getId());
194+
stmt.setInt(6, stat.getId());
201195
stmt.addBatch();
202196
}
203197

@@ -216,18 +210,17 @@ private void insert(Collection<PlayerStats> stats) {
216210
try (Connection conn = dataSource.getConnection();
217211
PreparedStatement stmt = conn.prepareStatement("INSERT INTO player_stats "
218212
+ "(uuid, playername, kills, deaths, killstreak, mobkills, last_online) VALUES "
219-
+ "(?, ?, ?, ?, ?, ?, ?)", Statement.RETURN_GENERATED_KEYS)) {
213+
+ "(?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)", Statement.RETURN_GENERATED_KEYS)) {
220214
conn.setAutoCommit(false);
221215
for (PlayerStats stat : stats) {
222-
stmt.setString(1, stat.getUuid() == null ? null : stat.getUuid().toString());
216+
stmt.setString(1, stat.getUuid().toString());
223217
stmt.setString(2, stat.getPlayername());
224218

225219
stmt.setInt(3, stat.getKills());
226220
stmt.setInt(4, stat.getDeaths());
227221
stmt.setInt(5, stat.getKillstreak());
228222
stmt.setInt(6, stat.getMobkills());
229223

230-
stmt.setTimestamp(7, Timestamp.from(stat.getLastOnlineDate()));
231224
stmt.addBatch();
232225
}
233226

@@ -292,7 +285,7 @@ public void setupDatabase() {
292285
+ "deaths integer NOT NULL, "
293286
+ "mobkills integer NOT NULL, "
294287
+ "killstreak integer NOT NULL, "
295-
+ "last_online timestamp NOT NULL )";
288+
+ "last_online timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP )";
296289

297290
if (dbConfig.getServerConfig().getDriverClassName().contains("sqlite")) {
298291
createTableQuery = createTableQuery.replace("AUTO_INCREMENT", "");
@@ -302,14 +295,11 @@ public void setupDatabase() {
302295
stmt.execute(createTableQuery);
303296
} catch (Exception ex) {
304297
logger.error("Error creating database ", ex);
298+
return;
305299
}
306300

307301
Bukkit.getScheduler().runTaskTimerAsynchronously(plugin, this::updateTopList, 20 * 60 * 5, 0);
308302
Bukkit.getScheduler().runTaskTimerAsynchronously(plugin, () -> {
309-
if (dataSource == null) {
310-
return;
311-
}
312-
313303
Future<Collection<? extends Player>> syncPlayers = Bukkit.getScheduler()
314304
.callSyncMethod(plugin, Bukkit::getOnlinePlayers);
315305

@@ -342,7 +332,7 @@ public void setupDatabase() {
342332
*/
343333
public Iterable<Entry<String, Integer>> getTop() {
344334
synchronized (toplist) {
345-
return toplist.entrySet();
335+
return ImmutableMap.copyOf(toplist).entrySet();
346336
}
347337
}
348338

@@ -421,6 +411,8 @@ private void registerEvents() {
421411

422412
private Replacer newVariable(String variable, Function<PlayerStats, Integer> fct) {
423413
return new Replacer(plugin, "kills")
424-
.scoreSupply(player -> getStats(player).map(fct).orElse(-1));
414+
.scoreSupply(player -> getStats(player)
415+
.map(fct)
416+
.orElse(-1));
425417
}
426418
}

‎pvp/src/main/java/com/github/games647/scoreboardstats/pvp/StatsLoader.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import java.lang.ref.WeakReference;
66
import java.time.Instant;
7-
import java.util.Optional;
87

98
import org.apache.commons.lang.builder.ToStringBuilder;
109
import org.bukkit.Bukkit;
@@ -42,12 +41,8 @@ public void run() {
4241
final Player player = weakPlayer.get();
4342
Database statsDatabase = weakDatabase.get();
4443
if (player != null && statsDatabase != null) {
45-
Optional<PlayerStats> optStats = statsDatabase.loadAccount(player);
46-
if (!optStats.isPresent()) {
47-
return;
48-
}
49-
50-
final PlayerStats stats = optStats.get();
44+
PlayerStats stats = statsDatabase.loadAccount(player)
45+
.orElse(new PlayerStats(player.getUniqueId(), player.getName()));
5146

5247
//update player name on every load, because it's changeable
5348
stats.setPlayername(player.getName());

‎variables/pom.xml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,20 @@
1010

1111
<artifactId>scoreboardstats-variables</artifactId>
1212
<packaging>jar</packaging>
13+
14+
<build>
15+
<plugins>
16+
<plugin>
17+
<groupId>org.apache.maven.plugins</groupId>
18+
<artifactId>maven-javadoc-plugin</artifactId>
19+
<version>3.0.0-M1</version>
20+
<configuration>
21+
<links>
22+
<link>https://hub.spigotmc.org/javadocs/spigot/</link>
23+
<link>https://docs.oracle.com/javase/9/docs/api/</link>
24+
</links>
25+
</configuration>
26+
</plugin>
27+
</plugins>
28+
</build>
1329
</project>

‎variables/src/main/java/com/github/games647/scoreboardstats/variables/Replacer.java‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -90,29 +90,27 @@ public <T extends Event> Replacer eventScore(Class<T> eventClass, IntSupplier su
9090
}
9191

9292
public <T extends Event> Replacer event(Class<T> eventClass, Function<T, String> function) {
93-
String eventName = eventClass.getCanonicalName();
94-
EventReplacer<?> replacer = eventsReplacers.get(eventName);
95-
if (replacer == null) {
96-
EventReplacer<T> checkedReplacer = new EventReplacer<>(this, eventClass);
97-
checkedReplacer.addFct(function);
98-
} else {
99-
((EventReplacer<T>) replacer).addFct(function);
100-
}
101-
93+
EventReplacer<T> checkedReplacer = getEventReplacer(eventClass);
94+
checkedReplacer.addFct(function);
10295
return this;
10396
}
10497

10598
public <T extends Event> Replacer eventScore(Class<T> eventClass, Function<T, Integer> function) {
99+
EventReplacer<T> checkedReplacer = getEventReplacer(eventClass);
100+
checkedReplacer.addScoreFct(function);
101+
return this;
102+
}
103+
104+
private <T extends Event> EventReplacer<T> getEventReplacer(Class<T> eventClass) {
106105
String eventName = eventClass.getCanonicalName();
107106
EventReplacer<?> replacer = eventsReplacers.get(eventName);
107+
108+
EventReplacer<T> checkedReplacer;
108109
if (replacer == null) {
109-
EventReplacer<T> checkedReplacer = new EventReplacer<>(this, eventClass);
110-
checkedReplacer.addScoreFct(function);
111-
} else {
112-
((EventReplacer<T>) replacer).addScoreFct(function);
110+
return new EventReplacer<>(this, eventClass);
113111
}
114112

115-
return this;
113+
return ((EventReplacer<T>) replacer);
116114
}
117115

118116
public String replace(Player player) {

‎variables/src/main/java/com/github/games647/scoreboardstats/variables/ReplacerAPI.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,7 @@ public void forceUpdate(String variable, String value) {
7070
public abstract void forceUpdate(Player player, String variable, String value);
7171

7272
public Optional<String> replace(Player player, String variable, boolean complete) throws ReplacerException {
73-
Replacer replacer = this.replacers.get(variable);
74-
if (replacer == null) {
75-
throw new UnknownVariableException(variable);
76-
}
77-
73+
Replacer replacer = getReplacer(variable);
7874
if (!complete && replacer.isGlobal() || replacer.isEventVariable() || replacer.isConstant()) {
7975
return Optional.empty();
8076
}
@@ -87,11 +83,7 @@ public Optional<String> replace(Player player, String variable, boolean complete
8783
}
8884

8985
public OptionalInt scoreReplace(Player player, String variable, boolean complete) throws ReplacerException {
90-
Replacer replacer = this.replacers.get(variable);
91-
if (replacer == null) {
92-
throw new UnknownVariableException(variable);
93-
}
94-
86+
Replacer replacer = getReplacer(variable);
9587
if (!complete && replacer.isGlobal() || replacer.isEventVariable() || replacer.isConstant()) {
9688
return OptionalInt.empty();
9789
}
@@ -102,4 +94,13 @@ public OptionalInt scoreReplace(Player player, String variable, boolean complete
10294
throw new ReplacerException(ex);
10395
}
10496
}
97+
98+
private Replacer getReplacer(String variable) throws UnknownVariableException {
99+
Replacer replacer = this.replacers.get(variable);
100+
if (replacer == null) {
101+
throw new UnknownVariableException(variable);
102+
}
103+
104+
return replacer;
105+
}
105106
}

0 commit comments

Comments
 (0)