From 161b47ee75ad6be159cc34c01e30d6bc434ec5d5 Mon Sep 17 00:00:00 2001 From: Bartosz Skrzypczak Date: Sun, 27 Aug 2017 22:58:55 +0200 Subject: [PATCH] Fix FML handshake crash, fixes #4285 and #3974 (#4304) --- .../handshake/FMLHandshakeClientState.java | 59 +++++++++++-------- .../handshake/FMLHandshakeServerState.java | 28 ++++----- .../handshake/HandshakeMessageHandler.java | 20 ++++--- .../network/handshake/IHandshakeState.java | 10 +++- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeClientState.java b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeClientState.java index 0e0f4001c4..4fd0bb18aa 100644 --- a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeClientState.java +++ b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeClientState.java @@ -24,6 +24,7 @@ import io.netty.channel.ChannelHandlerContext; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; @@ -55,26 +56,35 @@ enum FMLHandshakeClientState implements IHandshakeState START { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(HELLO); NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); dispatcher.clientListenForServerHandshake(); - return HELLO; } }, HELLO { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + boolean isVanilla = msg == null; + if (isVanilla) + { + cons.accept(DONE); + } + else + { + cons.accept(WAITINGSERVERDATA); + } // write our custom packet registration, always ctx.writeAndFlush(FMLHandshakeMessage.makeCustomChannelRegistration(NetworkRegistry.INSTANCE.channelNamesFor(Side.CLIENT))); - if (msg == null) + if (isVanilla) { NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); dispatcher.abortClientHandshake("VANILLA"); // VANILLA login - return DONE; + return; } ServerHello serverHelloPacket = (FMLHandshakeMessage.ServerHello)msg; @@ -87,37 +97,38 @@ enum FMLHandshakeClientState implements IHandshakeState } ctx.writeAndFlush(new FMLHandshakeMessage.ClientHello()).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); ctx.writeAndFlush(new FMLHandshakeMessage.ModList(Loader.instance().getActiveModList())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); - return WAITINGSERVERDATA; } }, WAITINGSERVERDATA { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { String result = FMLNetworkHandler.checkModList((FMLHandshakeMessage.ModList) msg, Side.SERVER); if (result != null) { + cons.accept(ERROR); NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); dispatcher.rejectHandshake(result); - return ERROR; + return; } - ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); if (!ctx.channel().attr(NetworkDispatcher.IS_LOCAL).get()) { - return WAITINGSERVERCOMPLETE; + cons.accept(WAITINGSERVERCOMPLETE); } else { - return PENDINGCOMPLETE; + cons.accept(PENDINGCOMPLETE); } + ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); + } }, WAITINGSERVERCOMPLETE { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { FMLHandshakeMessage.RegistryData pkt = (FMLHandshakeMessage.RegistryData)msg; Map snap = ctx.channel().attr(NetworkDispatcher.FML_GAMEDATA_SNAPSHOT).get(); @@ -135,8 +146,9 @@ enum FMLHandshakeClientState implements IHandshakeState if (pkt.hasMore()) { + cons.accept(WAITINGSERVERCOMPLETE); FMLLog.log.debug("Received Mod Registry mapping for {}: {} IDs {} overrides {} dummied", pkt.getName(), entry.ids.size(), entry.overrides.size(), entry.dummied.size()); - return WAITINGSERVERCOMPLETE; + return; } ctx.channel().attr(NetworkDispatcher.FML_GAMEDATA_SNAPSHOT).set(null); @@ -144,57 +156,56 @@ enum FMLHandshakeClientState implements IHandshakeState Multimap locallyMissing = GameData.injectSnapshot(snap, false, false); if (!locallyMissing.isEmpty()) { + cons.accept(ERROR); NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); dispatcher.rejectHandshake("Fatally missing registry entries"); FMLLog.log.fatal("Failed to connect to server: there are {} missing registry items", locallyMissing.size()); locallyMissing.asMap().forEach((key, value) -> FMLLog.log.debug("Missing {} Entries: {}", key, value)); - return ERROR; + return; } + cons.accept(PENDINGCOMPLETE); ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); - return PENDINGCOMPLETE; } }, PENDINGCOMPLETE { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(COMPLETE); ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); - return COMPLETE; } }, COMPLETE { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(DONE); NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); dispatcher.completeClientHandshake(); FMLMessage.CompleteHandshake complete = new FMLMessage.CompleteHandshake(Side.CLIENT); ctx.fireChannelRead(complete); ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); - return DONE; } }, DONE { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { if (msg instanceof FMLHandshakeMessage.HandshakeReset) { + cons.accept(HELLO); GameData.revertToFrozen(); - return HELLO; } - return this; } }, ERROR { @Override - public FMLHandshakeClientState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { - return this; } }; } diff --git a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeServerState.java b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeServerState.java index d8706ccc52..500daff10a 100644 --- a/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeServerState.java +++ b/src/main/java/net/minecraftforge/fml/common/network/handshake/FMLHandshakeServerState.java @@ -25,6 +25,7 @@ import io.netty.channel.ChannelHandlerContext; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.function.Consumer; import net.minecraft.util.ResourceLocation; import net.minecraftforge.fml.common.FMLLog; @@ -41,25 +42,25 @@ enum FMLHandshakeServerState implements IHandshakeState START { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(HELLO); NetworkDispatcher dispatcher = ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(); int overrideDim = dispatcher.serverInitiateHandshake(); ctx.writeAndFlush(FMLHandshakeMessage.makeCustomChannelRegistration(NetworkRegistry.INSTANCE.channelNamesFor(Side.SERVER))).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); ctx.writeAndFlush(new FMLHandshakeMessage.ServerHello(overrideDim)).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); - return HELLO; } }, HELLO { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { // Hello packet first if (msg instanceof FMLHandshakeMessage.ClientHello) { FMLLog.log.info("Client protocol version {}", Integer.toHexString(((FMLHandshakeMessage.ClientHello)msg).protocolVersion())); - return this; + return; } FMLHandshakeMessage.ModList client = (FMLHandshakeMessage.ModList)msg; @@ -69,18 +70,20 @@ enum FMLHandshakeServerState implements IHandshakeState String result = FMLNetworkHandler.checkModList(client, Side.CLIENT); if (result != null) { + cons.accept(ERROR); dispatcher.rejectHandshake(result); - return ERROR; + return; } + cons.accept(WAITINGCACK); ctx.writeAndFlush(new FMLHandshakeMessage.ModList(Loader.instance().getActiveModList())); - return WAITINGCACK; } }, WAITINGCACK { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(COMPLETE); if (!ctx.channel().attr(NetworkDispatcher.IS_LOCAL).get()) { Map snapshot = RegistryManager.ACTIVE.takeSnapshot(false); @@ -93,35 +96,32 @@ enum FMLHandshakeServerState implements IHandshakeState } ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); NetworkRegistry.INSTANCE.fireNetworkHandshake(ctx.channel().attr(NetworkDispatcher.FML_DISPATCHER).get(), Side.SERVER); - return COMPLETE; } }, COMPLETE { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { + cons.accept(DONE); // Poke the client ctx.writeAndFlush(new FMLHandshakeMessage.HandshakeAck(ordinal())).addListener(ChannelFutureListener.FIRE_EXCEPTION_ON_FAILURE); FMLMessage.CompleteHandshake complete = new FMLMessage.CompleteHandshake(Side.SERVER); ctx.fireChannelRead(complete); - return DONE; } }, DONE { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { - return this; } }, ERROR { @Override - public FMLHandshakeServerState accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg) + public void accept(ChannelHandlerContext ctx, FMLHandshakeMessage msg, Consumer cons) { - return this; } }; } diff --git a/src/main/java/net/minecraftforge/fml/common/network/handshake/HandshakeMessageHandler.java b/src/main/java/net/minecraftforge/fml/common/network/handshake/HandshakeMessageHandler.java index c0fec98641..0cded94a62 100644 --- a/src/main/java/net/minecraftforge/fml/common/network/handshake/HandshakeMessageHandler.java +++ b/src/main/java/net/minecraftforge/fml/common/network/handshake/HandshakeMessageHandler.java @@ -43,26 +43,30 @@ public class HandshakeMessageHandler & IHandshakeState> ext @Override protected void channelRead0(ChannelHandlerContext ctx, FMLHandshakeMessage msg) throws Exception { - S state = ctx.attr(fmlHandshakeState).get(); + S state = ctx.channel().attr(fmlHandshakeState).get(); FMLLog.log.debug("{}: {}->{}:{}", stateType.getSimpleName(), msg.toString(stateType), state.getClass().getName().substring(state.getClass().getName().lastIndexOf('.')+1), state); - S newState = state.accept(ctx, msg); - FMLLog.log.debug(" Next: {}", newState.name()); - ctx.attr(fmlHandshakeState).set(newState); + state.accept(ctx, msg, s -> + { + FMLLog.log.debug(" Next: {}", s.name()); + ctx.channel().attr(fmlHandshakeState).set(s); + }); } @Override public void channelActive(ChannelHandlerContext ctx) throws Exception { - ctx.attr(fmlHandshakeState).set(initialState); + ctx.channel().attr(fmlHandshakeState).set(initialState); } @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { S state = ctx.attr(fmlHandshakeState).get(); FMLLog.log.debug("{}: null->{}:{}", stateType.getSimpleName(), state.getClass().getName().substring(state.getClass().getName().lastIndexOf('.')+1), state); - S newState = state.accept(ctx, null); - FMLLog.log.debug(" Next: {}", newState.name()); - ctx.attr(fmlHandshakeState).set(newState); + state.accept(ctx, null, s -> + { + FMLLog.log.debug(" Next: {}", s.name()); + ctx.channel().attr(fmlHandshakeState).set(s); + }); } @Override diff --git a/src/main/java/net/minecraftforge/fml/common/network/handshake/IHandshakeState.java b/src/main/java/net/minecraftforge/fml/common/network/handshake/IHandshakeState.java index 6210127dc2..67ec28f86d 100644 --- a/src/main/java/net/minecraftforge/fml/common/network/handshake/IHandshakeState.java +++ b/src/main/java/net/minecraftforge/fml/common/network/handshake/IHandshakeState.java @@ -21,8 +21,16 @@ package net.minecraftforge.fml.common.network.handshake; import io.netty.channel.ChannelHandlerContext; +import java.util.function.Consumer; + import javax.annotation.Nullable; public interface IHandshakeState { - S accept(ChannelHandlerContext ctx, @Nullable FMLHandshakeMessage msg); + /** + * Accepts FML handshake message for this state, and if needed - switches to another handshake state + * using the provided consumer. + * + * The consumer allows to set new state before sending any messages to avoid race conditions. + */ + void accept(ChannelHandlerContext ctx, @Nullable FMLHandshakeMessage msg, Consumer cons); }