GeyserMC-Mirror/Geyser
1
0
Fork 0
mirror of https://github.com/GeyserMC/Geyser.git synced 2026-01-04 15:31:36 +00:00
Code Issues Releases Activity

Fix the remaining concurrency issues with scoreboard display slots (#5592)

Browse Source 
With the scoreboard updater thread, there can be multiple threads editing specific bits which can result in concurrency issues. This PR aims to remove those remaining concurrency issues.
This commit is contained in:
Tim203
2025-06-14 15:36:39 +02:00
committed by GitHub
parent b3716c77b1
commit 17baaf6721
4 changed files with 107 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
core/src/main/java/org/geysermc/geyser/scoreboard/display/slot/BelownameDisplaySlot.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 GeyserMC. http://geysermc.org
* Copyright (c) 2024-2025 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -61,44 +61,42 @@ public class BelownameDisplaySlot extends DisplaySlot {
// remove is handled in #remove()
if (updateType == UpdateType.ADD) {
for (PlayerEntity player : session.getEntityCache().getAllPlayerEntities()) {
playerRegistered(player);
}
session.getEntityCache().forEachPlayerEntity(this::playerRegistered);
return;
}
if (updateType == UpdateType.UPDATE) {
for (PlayerEntity player : session.getEntityCache().getAllPlayerEntities()) {
session.getEntityCache().forEachPlayerEntity(player -> {
setBelowNameText(player, scoreFor(player.getUsername()));
}
});
updateType = UpdateType.NOTHING;
return;
}
for (var score : displayScores.values()) {
// we don't have to worry about a score not existing, because that's handled by both
// this method when an objective is added and addScore/playerRegistered.
// we only have to update them, if they have changed
// (or delete them, if the score no longer exists)
if (!score.shouldUpdate()) {
continue;
}
synchronized (displayScores) {
for (var score : displayScores.values()) {
// we don't have to worry about a score not existing, because that's handled by both
// this method when an objective is added and addScore/playerRegistered.
// we only have to update them, if they have changed
// (or delete them, if the score no longer exists)
if (!score.shouldUpdate()) {
continue;
}
if (score.referenceRemoved()) {
clearBelowNameText(score.player());
continue;
}
if (score.referenceRemoved()) {
clearBelowNameText(score.player());
continue;
}
score.markUpdated();
setBelowNameText(score.player(), score.reference());
score.markUpdated();
setBelowNameText(score.player(), score.reference());
}
}
}
@Override
public void remove() {
updateType = UpdateType.REMOVE;
for (PlayerEntity player : session.getEntityCache().getAllPlayerEntities()) {
clearBelowNameText(player);
}
session.getEntityCache().forEachPlayerEntity(this::clearBelowNameText);
}
@Override
@@ -119,7 +117,9 @@ public class BelownameDisplaySlot extends DisplaySlot {
@Override
public void playerRemoved(PlayerEntity player) {
displayScores.remove(player.getGeyserId());
synchronized (displayScores) {
displayScores.remove(player.getGeyserId());
}
}
private void addDisplayScore(ScoreReference reference) {
@@ -131,7 +131,9 @@ public class BelownameDisplaySlot extends DisplaySlot {
private BelownameDisplayScore addDisplayScore(PlayerEntity player, ScoreReference reference) {
var score = new BelownameDisplayScore(this, objective.getScoreboard().nextId(), reference, player);
displayScores.put(player.getGeyserId(), score);
synchronized (displayScores) {
displayScores.put(player.getGeyserId(), score);
}
return score;
}

81
core/src/main/java/org/geysermc/geyser/scoreboard/display/slot/PlayerlistDisplaySlot.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 GeyserMC. http://geysermc.org
* Copyright (c) 2024-2025 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -26,7 +26,6 @@
package org.geysermc.geyser.scoreboard.display.slot;
import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
import it.unimi.dsi.fastutil.longs.Long2ObjectMaps;
import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
import java.util.ArrayList;
import java.util.Collections;
@@ -41,8 +40,7 @@ import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.mcprotocollib.protocol.data.game.scoreboard.ScoreboardPosition;
public class PlayerlistDisplaySlot extends DisplaySlot {
private final Long2ObjectMap<PlayerlistDisplayScore> displayScores =
Long2ObjectMaps.synchronize(new Long2ObjectOpenHashMap<>());
private final Long2ObjectMap<PlayerlistDisplayScore> displayScores = new Long2ObjectOpenHashMap<>();
private final List<PlayerlistDisplayScore> removedScores = Collections.synchronizedList(new ArrayList<>());
public PlayerlistDisplaySlot(GeyserSession session, Objective objective) {
@@ -71,35 +69,37 @@ public class PlayerlistDisplaySlot extends DisplaySlot {
removedScores.clear();
}
for (var score : displayScores.values()) {
if (score.referenceRemoved()) {
ScoreInfo cachedInfo = score.cachedInfo();
// cachedInfo can be null here when ScoreboardUpdater is being used and a score is added and
// removed before a single update cycle is performed
if (cachedInfo != null) {
removeScores.add(cachedInfo);
synchronized (displayScores) {
for (var score : displayScores.values()) {
if (score.referenceRemoved()) {
ScoreInfo cachedInfo = score.cachedInfo();
// cachedInfo can be null here when ScoreboardUpdater is being used and a score is added and
// removed before a single update cycle is performed
if (cachedInfo != null) {
removeScores.add(cachedInfo);
}
continue;
}
continue;
}
//todo does an animated title exist on tab?
boolean add = objectiveAdd || objectiveUpdate;
boolean exists = score.exists();
//todo does an animated title exist on tab?
boolean add = objectiveAdd || objectiveUpdate;
boolean exists = score.exists();
if (score.shouldUpdate()) {
score.update(objective);
add = true;
}
if (score.shouldUpdate()) {
score.update(objective);
add = true;
}
if (add) {
addScores.add(score.cachedInfo());
}
if (add) {
addScores.add(score.cachedInfo());
}
// we need this as long as MCPE-143063 hasn't been fixed.
// the checks after 'add' are there to prevent removing scores that
// are going to be removed anyway / don't need to be removed
if (add && exists && objectiveNothing) {
removeScores.add(score.cachedInfo());
// we need this as long as MCPE-143063 hasn't been fixed.
// the checks after 'add' are there to prevent removing scores that
// are going to be removed anyway / don't need to be removed
if (add && exists && objectiveNothing) {
removeScores.add(score.cachedInfo());
}
}
}
@@ -124,16 +124,17 @@ public class PlayerlistDisplaySlot extends DisplaySlot {
players.add(selfPlayer);
}
for (PlayerEntity player : players) {
var score =
new PlayerlistDisplayScore(this, objective.getScoreboard().nextId(), reference, player.getGeyserId());
displayScores.put(player.getGeyserId(), score);
synchronized (displayScores) {
for (PlayerEntity player : players) {
var score = new PlayerlistDisplayScore(this, objective.getScoreboard().nextId(), reference, player.getGeyserId());
displayScores.put(player.getGeyserId(), score);
}
}
}
private void registerExisting() {
playerRegistered(session.getPlayerEntity());
session.getEntityCache().getAllPlayerEntities().forEach(this::playerRegistered);
session.getEntityCache().forEachPlayerEntity(this::playerRegistered);
}
@Override
@@ -142,14 +143,20 @@ public class PlayerlistDisplaySlot extends DisplaySlot {
if (reference == null) {
return;
}
var score =
new PlayerlistDisplayScore(this, objective.getScoreboard().nextId(), reference, player.getGeyserId());
displayScores.put(player.getGeyserId(), score);
var score = new PlayerlistDisplayScore(this, objective.getScoreboard().nextId(), reference, player.getGeyserId());
synchronized (displayScores) {
displayScores.put(player.getGeyserId(), score);
}
}
@Override
public void playerRemoved(PlayerEntity player) {
var score = displayScores.remove(player.getGeyserId());
PlayerlistDisplayScore score;
synchronized (displayScores) {
score = displayScores.remove(player.getGeyserId());
}
if (score == null) {
return;
}

46
core/src/main/java/org/geysermc/geyser/session/cache/EntityCache.java vendored
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022 GeyserMC. http://geysermc.org
* Copyright (c) 2019-2025 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -32,11 +32,11 @@ import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.objects.ObjectArrayList;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
import lombok.Getter;
import org.geysermc.geyser.entity.type.Entity;
import org.geysermc.geyser.entity.type.Tickable;
@@ -136,10 +136,12 @@ public class EntityCache {
}
public void addPlayerEntity(PlayerEntity entity) {
// putIfAbsent matches the behavior of playerInfoMap in Java as of 1.19.3
boolean exists = playerEntities.putIfAbsent(entity.getUuid(), entity) != null;
if (exists) {
return;
synchronized (playerEntities) {
// putIfAbsent matches the behavior of playerInfoMap in Java as of 1.19.3
boolean exists = playerEntities.putIfAbsent(entity.getUuid(), entity) != null;
if (exists) {
return;
}
}
// notify scoreboard for new entity
@@ -148,21 +150,29 @@ public class EntityCache {
}
public PlayerEntity getPlayerEntity(UUID uuid) {
return playerEntities.get(uuid);
synchronized (playerEntities) {
return playerEntities.get(uuid);
}
}
public List<PlayerEntity> getPlayersByName(String name) {
var list = new ArrayList<PlayerEntity>();
for (PlayerEntity player : playerEntities.values()) {
if (name.equals(player.getUsername())) {
list.add(player);
synchronized (playerEntities) {
for (PlayerEntity player : playerEntities.values()) {
if (name.equals(player.getUsername())) {
list.add(player);
}
}
}
return list;
}
public PlayerEntity removePlayerEntity(UUID uuid) {
var player = playerEntities.remove(uuid);
PlayerEntity player;
synchronized (playerEntities) {
player = playerEntities.remove(uuid);
}
if (player != null) {
// notify scoreboard
session.getWorldCache().getScoreboard().playerRemoved(player);
@@ -170,12 +180,20 @@ public class EntityCache {
return player;
}
public Collection<PlayerEntity> getAllPlayerEntities() {
return playerEntities.values();
/**
* Run a specific bit of code for each cached player entity.
* As usual with synchronized, try to minimize the amount of work you because you block the PlayerList collection.
*/
public void forEachPlayerEntity(Consumer<PlayerEntity> player) {
synchronized (playerEntities) {
playerEntities.values().forEach(player);
}
}
public void removeAllPlayerEntities() {
playerEntities.clear();
synchronized (playerEntities) {
playerEntities.clear();
}
}
public void addBossBar(UUID uuid, BossBar bossBar) {

9
core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaFinishConfigurationTranslator.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2024 GeyserMC. http://geysermc.org
* Copyright (c) 2024-2025 GeyserMC. http://geysermc.org
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
@@ -29,7 +29,6 @@ import org.cloudburstmc.protocol.bedrock.data.inventory.crafting.recipe.MultiRec
import org.cloudburstmc.protocol.bedrock.data.inventory.crafting.recipe.RecipeData;
import org.cloudburstmc.protocol.bedrock.packet.CraftingDataPacket;
import org.cloudburstmc.protocol.bedrock.packet.PlayerListPacket;
import org.geysermc.geyser.entity.type.player.PlayerEntity;
import org.geysermc.geyser.registry.Registries;
import org.geysermc.geyser.session.GeyserSession;
import org.geysermc.geyser.translator.protocol.PacketTranslator;
@@ -59,9 +58,9 @@ public class JavaFinishConfigurationTranslator extends PacketTranslator<Clientbo
public void translate(GeyserSession session, ClientboundFinishConfigurationPacket packet) {
// Clear the player list, as on Java the player list is cleared after transitioning from config to play phase
List<PlayerListPacket.Entry> entries = new ArrayList<>();
for (PlayerEntity otherEntity : session.getEntityCache().getAllPlayerEntities()) {
entries.add(new PlayerListPacket.Entry(otherEntity.getTabListUuid()));
}
session.getEntityCache().forEachPlayerEntity(otherPlayer -> {
entries.add(new PlayerListPacket.Entry(otherPlayer.getTabListUuid()));
});
PlayerListUtils.batchSendPlayerList(session, entries, PlayerListPacket.Action.REMOVE);
session.getEntityCache().removeAllPlayerEntities();