From d61a598c94cd6b88e7366a5c0fc03b04298f0bcb Mon Sep 17 00:00:00 2001 From: Felix Cravic Date: Wed, 2 Dec 2020 21:28:36 +0100 Subject: [PATCH] Prevent players being disconnected two times during a clean stop, also made shutdown tasks being executed in a single thread --- .../net/minestom/server/MinecraftServer.java | 12 +++ .../net/minestom/server/entity/Entity.java | 2 +- .../net/minestom/server/entity/Player.java | 97 ++++--------------- .../server/network/ConnectionManager.java | 47 +++++++++ .../server/network/netty/NettyServer.java | 2 - .../server/timer/SchedulerManager.java | 2 +- .../java/net/minestom/server/timer/Task.java | 9 ++ 7 files changed, 89 insertions(+), 82 deletions(-) diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index 4ebbdaa6d..71501673a 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -124,6 +124,7 @@ public final class MinecraftServer { // Data private static boolean initialized; private static boolean started; + private static boolean stopping; private static int chunkViewDistance = 8; private static int entityViewDistance = 5; @@ -432,6 +433,15 @@ public final class MinecraftServer { return started; } + /** + * Gets if the server is currently being shutdown using {@link #stopCleanly()}. + * + * @return true if the server is being stopped + */ + public static boolean isStopping() { + return stopping; + } + /** * Gets the chunk view distance of the server. * @@ -721,9 +731,11 @@ public final class MinecraftServer { * Stops this server properly (saves if needed, kicking players, etc.) */ public static void stopCleanly() { + stopping = true; LOGGER.info("Stopping Minestom server."); updateManager.stop(); schedulerManager.shutdown(); + connectionManager.shutdown(); nettyServer.stop(); storageManager.getLoadedLocations().forEach(StorageLocation::close); LOGGER.info("Shutting down all thread pools."); diff --git a/src/main/java/net/minestom/server/entity/Entity.java b/src/main/java/net/minestom/server/entity/Entity.java index 7fde0ccc1..d341527ee 100644 --- a/src/main/java/net/minestom/server/entity/Entity.java +++ b/src/main/java/net/minestom/server/entity/Entity.java @@ -596,7 +596,7 @@ public abstract class Entity implements Viewable, EventHandler, DataContainer, P sendSynchronization(); } - if (shouldRemove()) { + if (shouldRemove() && !MinecraftServer.isStopping()) { remove(); } } diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index acb496ab5..073262ece 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/src/main/java/net/minestom/server/entity/Player.java @@ -1,21 +1,5 @@ package net.minestom.server.entity; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.Hashtable; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.UUID; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.CopyOnWriteArraySet; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.Consumer; - -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - import net.minestom.server.MinecraftServer; import net.minestom.server.advancements.AdvancementTab; import net.minestom.server.attribute.Attribute; @@ -37,16 +21,7 @@ import net.minestom.server.event.inventory.InventoryOpenEvent; import net.minestom.server.event.item.ItemDropEvent; import net.minestom.server.event.item.ItemUpdateStateEvent; import net.minestom.server.event.item.PickupExperienceEvent; -import net.minestom.server.event.player.PlayerChunkLoadEvent; -import net.minestom.server.event.player.PlayerChunkUnloadEvent; -import net.minestom.server.event.player.PlayerDisconnectEvent; -import net.minestom.server.event.player.PlayerEatEvent; -import net.minestom.server.event.player.PlayerPreLoginEvent; -import net.minestom.server.event.player.PlayerRespawnEvent; -import net.minestom.server.event.player.PlayerSkinInitEvent; -import net.minestom.server.event.player.PlayerSpawnEvent; -import net.minestom.server.event.player.PlayerTickEvent; -import net.minestom.server.event.player.UpdateTagListEvent; +import net.minestom.server.event.player.*; import net.minestom.server.gamedata.tags.TagManager; import net.minestom.server.instance.Chunk; import net.minestom.server.instance.Instance; @@ -61,49 +36,7 @@ import net.minestom.server.network.PlayerProvider; import net.minestom.server.network.packet.client.ClientPlayPacket; import net.minestom.server.network.packet.client.play.ClientChatMessagePacket; import net.minestom.server.network.packet.server.ServerPacket; -import net.minestom.server.network.packet.server.play.BlockBreakAnimationPacket; -import net.minestom.server.network.packet.server.play.CameraPacket; -import net.minestom.server.network.packet.server.play.ChangeGameStatePacket; -import net.minestom.server.network.packet.server.play.ChatMessagePacket; -import net.minestom.server.network.packet.server.play.CloseWindowPacket; -import net.minestom.server.network.packet.server.play.CombatEventPacket; -import net.minestom.server.network.packet.server.play.DeclareCommandsPacket; -import net.minestom.server.network.packet.server.play.DeclareRecipesPacket; -import net.minestom.server.network.packet.server.play.DestroyEntitiesPacket; -import net.minestom.server.network.packet.server.play.DisconnectPacket; -import net.minestom.server.network.packet.server.play.EffectPacket; -import net.minestom.server.network.packet.server.play.EntityEquipmentPacket; -import net.minestom.server.network.packet.server.play.EntityHeadLookPacket; -import net.minestom.server.network.packet.server.play.EntityMetaDataPacket; -import net.minestom.server.network.packet.server.play.EntityPositionAndRotationPacket; -import net.minestom.server.network.packet.server.play.EntityPositionPacket; -import net.minestom.server.network.packet.server.play.EntityRotationPacket; -import net.minestom.server.network.packet.server.play.EntitySoundEffect; -import net.minestom.server.network.packet.server.play.FacePlayerPacket; -import net.minestom.server.network.packet.server.play.HeldItemChangePacket; -import net.minestom.server.network.packet.server.play.JoinGamePacket; -import net.minestom.server.network.packet.server.play.KeepAlivePacket; -import net.minestom.server.network.packet.server.play.NamedSoundEffectPacket; -import net.minestom.server.network.packet.server.play.OpenWindowPacket; -import net.minestom.server.network.packet.server.play.PlayerAbilitiesPacket; -import net.minestom.server.network.packet.server.play.PlayerInfoPacket; -import net.minestom.server.network.packet.server.play.PlayerListHeaderAndFooterPacket; -import net.minestom.server.network.packet.server.play.PlayerPositionAndLookPacket; -import net.minestom.server.network.packet.server.play.PluginMessagePacket; -import net.minestom.server.network.packet.server.play.ResourcePackSendPacket; -import net.minestom.server.network.packet.server.play.RespawnPacket; -import net.minestom.server.network.packet.server.play.ServerDifficultyPacket; -import net.minestom.server.network.packet.server.play.SetExperiencePacket; -import net.minestom.server.network.packet.server.play.SoundEffectPacket; -import net.minestom.server.network.packet.server.play.SpawnPlayerPacket; -import net.minestom.server.network.packet.server.play.SpawnPositionPacket; -import net.minestom.server.network.packet.server.play.StopSoundPacket; -import net.minestom.server.network.packet.server.play.TagsPacket; -import net.minestom.server.network.packet.server.play.TitlePacket; -import net.minestom.server.network.packet.server.play.UnloadChunkPacket; -import net.minestom.server.network.packet.server.play.UnlockRecipesPacket; -import net.minestom.server.network.packet.server.play.UpdateHealthPacket; -import net.minestom.server.network.packet.server.play.UpdateViewPositionPacket; +import net.minestom.server.network.packet.server.play.*; import net.minestom.server.network.player.NettyPlayerConnection; import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.recipe.Recipe; @@ -128,6 +61,14 @@ import net.minestom.server.utils.time.TimeUnit; import net.minestom.server.utils.time.UpdateOption; import net.minestom.server.utils.validate.Check; import net.minestom.server.world.DimensionType; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; /** * Those are the major actors of the server, @@ -970,23 +911,23 @@ public class Player extends LivingEntity implements CommandSender { namedSoundEffectPacket.pitch = pitch; playerConnection.sendPacket(namedSoundEffectPacket); } - + /** * Plays a sound directly to the player (constant volume). * - * @param sound the sound to play + * @param sound the sound to play * @param soundCategory the sound category * @param volume the volume of the sound (1 is 100%) * @param pitch the pitch of the sound, between 0.5 and 2.0 */ public void playSound(@NotNull Sound sound, @NotNull SoundCategory soundCategory, float volume, float pitch) { - EntitySoundEffect entitySoundEffect = new EntitySoundEffect(); - entitySoundEffect.entityId = getEntityId(); - entitySoundEffect.soundId = sound.getId(); - entitySoundEffect.soundCategory = soundCategory; - entitySoundEffect.volume = volume; - entitySoundEffect.pitch = pitch; - playerConnection.sendPacket(entitySoundEffect); + EntitySoundEffect entitySoundEffect = new EntitySoundEffect(); + entitySoundEffect.entityId = getEntityId(); + entitySoundEffect.soundId = sound.getId(); + entitySoundEffect.soundCategory = soundCategory; + entitySoundEffect.volume = volume; + entitySoundEffect.pitch = pitch; + playerConnection.sendPacket(entitySoundEffect); } /** diff --git a/src/main/java/net/minestom/server/network/ConnectionManager.java b/src/main/java/net/minestom/server/network/ConnectionManager.java index 027fcfe87..9bb9d715c 100644 --- a/src/main/java/net/minestom/server/network/ConnectionManager.java +++ b/src/main/java/net/minestom/server/network/ConnectionManager.java @@ -1,5 +1,9 @@ package net.minestom.server.network; +import io.netty.channel.Channel; +import net.minestom.server.MinecraftServer; +import net.minestom.server.chat.ChatColor; +import net.minestom.server.chat.ColoredText; import net.minestom.server.chat.JsonMessage; import net.minestom.server.entity.Player; import net.minestom.server.entity.fakeplayer.FakePlayer; @@ -8,6 +12,8 @@ import net.minestom.server.listener.manager.ServerPacketConsumer; import net.minestom.server.network.packet.client.login.LoginStartPacket; import net.minestom.server.network.packet.server.login.LoginSuccessPacket; import net.minestom.server.network.packet.server.play.ChatMessagePacket; +import net.minestom.server.network.packet.server.play.DisconnectPacket; +import net.minestom.server.network.player.NettyPlayerConnection; import net.minestom.server.network.player.PlayerConnection; import net.minestom.server.utils.PacketUtils; import net.minestom.server.utils.callback.validator.PlayerValidator; @@ -38,6 +44,8 @@ public final class ConnectionManager { // The consumers to call once a player connect, mostly used to init events private final List> playerInitializations = new CopyOnWriteArrayList<>(); + private ColoredText shutdownText = ColoredText.of(ChatColor.RED, "The server is shutting down."); + /** * Gets the {@link Player} linked to a {@link PlayerConnection}. * @@ -255,6 +263,26 @@ public final class ConnectionManager { this.playerInitializations.add(playerInitialization); } + /** + * Gets the kick reason when the server is shutdown using {@link MinecraftServer#stopCleanly()}. + * + * @return the kick reason in case on a shutdown + */ + @NotNull + public ColoredText getShutdownText() { + return shutdownText; + } + + /** + * Changes the kick reason in case of a shutdown. + * + * @param shutdownText the new shutdown kick reason + * @see #getShutdownText() + */ + public void setShutdownText(@NotNull ColoredText shutdownText) { + this.shutdownText = shutdownText; + } + /** * Adds a new {@link Player} in the players list. * Is currently used at @@ -316,4 +344,23 @@ public final class ConnectionManager { connection.setConnectionState(ConnectionState.PLAY); return createPlayer(uuid, username, connection); } + + /** + * Shutdowns the connection manager by kicking all the currently connected players + */ + public void shutdown() { + DisconnectPacket disconnectPacket = new DisconnectPacket(); + disconnectPacket.message = getShutdownText(); + for (Player player : getOnlinePlayers()) { + final PlayerConnection playerConnection = player.getPlayerConnection(); + if (playerConnection instanceof NettyPlayerConnection) { + final NettyPlayerConnection nettyPlayerConnection = (NettyPlayerConnection) playerConnection; + final Channel channel = nettyPlayerConnection.getChannel(); + channel.writeAndFlush(disconnectPacket); + channel.close(); + } + } + this.players.clear(); + this.connectionPlayerMap.clear(); + } } diff --git a/src/main/java/net/minestom/server/network/netty/NettyServer.java b/src/main/java/net/minestom/server/network/netty/NettyServer.java index a821ce45c..87f878798 100644 --- a/src/main/java/net/minestom/server/network/netty/NettyServer.java +++ b/src/main/java/net/minestom/server/network/netty/NettyServer.java @@ -230,8 +230,6 @@ public final class NettyServer { * Stops the server and the various services. */ public void stop() { - // FIXME: fix "java.util.concurrent.RejectedExecutionException: event executor terminated" - // TODO: probably because we are doing IO after #stop() is executed try { this.serverChannel.close().sync(); this.worker.shutdownGracefully().await(); diff --git a/src/main/java/net/minestom/server/timer/SchedulerManager.java b/src/main/java/net/minestom/server/timer/SchedulerManager.java index b15e7fafe..d389178ba 100644 --- a/src/main/java/net/minestom/server/timer/SchedulerManager.java +++ b/src/main/java/net/minestom/server/timer/SchedulerManager.java @@ -98,7 +98,7 @@ public final class SchedulerManager { public void shutdown() { MinecraftServer.LOGGER.info("Executing all shutdown tasks.."); for (Task task : this.getShutdownTasks()) { - task.schedule(); + task.runRunnable(); } MinecraftServer.LOGGER.info("Shutting down the scheduled execution service and batches pool."); this.timerExecutionService.shutdown(); diff --git a/src/main/java/net/minestom/server/timer/Task.java b/src/main/java/net/minestom/server/timer/Task.java index f9899d0c5..eccc8cc71 100644 --- a/src/main/java/net/minestom/server/timer/Task.java +++ b/src/main/java/net/minestom/server/timer/Task.java @@ -73,6 +73,15 @@ public class Task implements Runnable { }); } + /** + * Executes the internal runnable. + *

+ * Should probably use {@link #schedule()} instead. + */ + public void runRunnable() { + this.runnable.run(); + } + /** * Sets up the task for correct execution. */