Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite some internal code to allow better disconnect handling #881

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ public void connected(ConnectedEvent event) {

@Override
public void disconnecting(DisconnectingEvent event) {
log.info("CLIENT Disconnecting: {}", event.getReason());
log.info("CLIENT Disconnecting: {}", event.getDetails().reason());
}

@Override
public void disconnected(DisconnectedEvent event) {
log.info("CLIENT Disconnected: {}", event.getReason());
log.info("CLIENT Disconnected: {}", event.getDetails().reason());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ public void connected(ConnectedEvent event) {

@Override
public void disconnecting(DisconnectingEvent event) {
log.info("SERVER Disconnecting: {}", event.getReason());
log.info("SERVER Disconnecting: {}", event.getDetails().reason());
}

@Override
public void disconnected(DisconnectedEvent event) {
log.info("SERVER Disconnected: {}", event.getReason());
log.info("SERVER Disconnected: {}", event.getDetails().reason());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
import org.geysermc.mcprotocollib.network.crypt.EncryptionConfig;
import org.geysermc.mcprotocollib.network.packet.DefaultPacketHeader;
import org.geysermc.mcprotocollib.network.packet.PacketHeader;
import org.geysermc.mcprotocollib.network.packet.PacketProtocol;
import org.geysermc.mcprotocollib.network.packet.PacketRegistry;
import org.geysermc.mcprotocollib.protocol.MinecraftProtocol;
import org.geysermc.mcprotocollib.protocol.codec.MinecraftTypes;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.crypto.SecretKey;
import java.security.GeneralSecurityException;

public class TestProtocol extends PacketProtocol {
public class TestProtocol extends MinecraftProtocol {
private static final Logger log = LoggerFactory.getLogger(TestProtocol.class);
private final PacketHeader header = new DefaultPacketHeader();
private final PacketRegistry registry = new PacketRegistry();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public void packetReceived(Session session, Packet packet) {

@Override
public void disconnected(DisconnectedEvent event) {
log.info("Disconnected: {}", event.getReason(), event.getCause());
log.info("Disconnected: {}", event.getDetails().reason(), event.getDetails().cause());
}
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.geysermc.mcprotocollib.network;

public enum PacketFlow {
SERVERBOUND,
CLIENTBOUND;

public PacketFlow opposite() {
return this == CLIENTBOUND ? SERVERBOUND : CLIENTBOUND;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
import org.geysermc.mcprotocollib.network.crypt.EncryptionConfig;
import org.geysermc.mcprotocollib.network.event.session.SessionEvent;
import org.geysermc.mcprotocollib.network.event.session.SessionListener;
import org.geysermc.mcprotocollib.network.netty.FlushHandler;
import org.geysermc.mcprotocollib.network.helper.DisconnectionDetails;
import org.geysermc.mcprotocollib.network.packet.Packet;
import org.geysermc.mcprotocollib.network.packet.PacketProtocol;
import org.geysermc.mcprotocollib.protocol.MinecraftProtocol;
import org.geysermc.mcprotocollib.protocol.data.ProtocolState;

import java.net.SocketAddress;
import java.util.List;
Expand Down Expand Up @@ -42,7 +43,7 @@ public interface Session {
*
* @return The session's packet protocol.
*/
PacketProtocol getPacketProtocol();
MinecraftProtocol getPacketProtocol();

/**
* Gets this session's set flags. If this session belongs to a server, the server's
Expand Down Expand Up @@ -204,7 +205,7 @@ default void send(@NonNull Packet packet) {
* @see #disconnect(String, Throwable)
*/
default void disconnect(@NonNull String reason) {
this.disconnect(reason, null);
this.disconnect(new DisconnectionDetails(Component.text(reason)));
}

/**
Expand All @@ -217,7 +218,7 @@ default void disconnect(@NonNull String reason) {
* @see #disconnect(Component, Throwable)
*/
default void disconnect(@NonNull String reason, @Nullable Throwable cause) {
this.disconnect(Component.text(reason), cause);
this.disconnect(new DisconnectionDetails(Component.text(reason), cause));
}

/**
Expand All @@ -226,7 +227,7 @@ default void disconnect(@NonNull String reason, @Nullable Throwable cause) {
* @param reason Reason for disconnecting.
*/
default void disconnect(@NonNull Component reason) {
this.disconnect(reason, null);
this.disconnect(new DisconnectionDetails(reason));
}

/**
Expand All @@ -235,13 +236,22 @@ default void disconnect(@NonNull Component reason) {
* @param reason Reason for disconnecting.
* @param cause Throwable responsible for disconnecting.
*/
void disconnect(@NonNull Component reason, @Nullable Throwable cause);
default void disconnect(@NonNull Component reason, @Nullable Throwable cause) {
this.disconnect(new DisconnectionDetails(reason, cause));
}

/**
* Disconnects the session.
*
* @param disconnectionDetails Info about disconnect.
*/
void disconnect(@NonNull DisconnectionDetails disconnectionDetails);

/**
* Auto read in netty means that the server is automatically reading from the channel.
* Turning it off means that we won't get more packets being decoded until we turn it back on.
* We use this to hold off on reading packets until we are ready to process them.
* For example this is used for switching inbound states with {@link #switchInboundState(Runnable)}.
* For example this is used for switching inbound states with {@link #switchInboundState(ProtocolState)}.
*
* @param autoRead Whether to enable auto read.
* Default is true.
Expand All @@ -266,25 +276,15 @@ default void disconnect(@NonNull Component reason) {
* Changes the inbound state of the session and then re-enables auto read.
* This is used after a terminal packet was handled and the session is ready to receive more packets in the new state.
*
* @param switcher The runnable that switches the inbound state.
* @param state The state to switch to.
*/
default void switchInboundState(Runnable switcher) {
switcher.run();

// We switched to the new inbound state
// we can start reading again
setAutoRead(true);
}
void switchInboundState(ProtocolState state);

/**
* Flushes all packets that are due to be sent and changes the outbound state of the session.
* This makes sure no other threads have scheduled packets to be sent.
*
* @param switcher The runnable that switches the outbound state.
* @param state The state to switch to.
*/
default void switchOutboundState(Runnable switcher) {
getChannel().writeAndFlush(FlushHandler.FLUSH_PACKET).syncUninterruptibly();

switcher.run();
}
void switchOutboundState(ProtocolState state);
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
package org.geysermc.mcprotocollib.network.event.session;

import net.kyori.adventure.text.Component;
import org.geysermc.mcprotocollib.network.Session;
import org.geysermc.mcprotocollib.network.helper.DisconnectionDetails;

/**
* Called when the session is disconnected.
*/
public class DisconnectedEvent implements SessionEvent {
private final Session session;
private final Component reason;
private final Throwable cause;
private final DisconnectionDetails disconnectionDetails;

/**
* Creates a new DisconnectedEvent instance.
*
* @param session Session being disconnected.
* @param reason Reason for the session to disconnect.
* @param cause Throwable that caused the disconnect.
* @param disconnectionDetails Reason for the disconnection.
*/
public DisconnectedEvent(Session session, Component reason, Throwable cause) {
public DisconnectedEvent(Session session, DisconnectionDetails disconnectionDetails) {
this.session = session;
this.reason = reason;
this.cause = cause;
this.disconnectionDetails = disconnectionDetails;
}

/**
Expand All @@ -38,17 +35,8 @@ public Session getSession() {
*
* @return The event's reason.
*/
public Component getReason() {
return this.reason;
}

/**
* Gets the Throwable responsible for the session disconnecting.
*
* @return The Throwable responsible for the disconnect, or null if the disconnect was not caused by a Throwable.
*/
public Throwable getCause() {
return this.cause;
public DisconnectionDetails getDetails() {
return this.disconnectionDetails;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,24 @@
package org.geysermc.mcprotocollib.network.event.session;

import net.kyori.adventure.text.Component;
import org.geysermc.mcprotocollib.network.Session;
import org.geysermc.mcprotocollib.network.helper.DisconnectionDetails;

/**
* Called when the session is about to disconnect.
*/
public class DisconnectingEvent implements SessionEvent {
private final Session session;
private final Component reason;
private final Throwable cause;
private final DisconnectionDetails disconnectionDetails;

/**
* Creates a new DisconnectingEvent instance.
*
* @param session Session being disconnected.
* @param reason Reason for the session to disconnect.
* @param cause Throwable that caused the disconnect.
* @param disconnectionDetails Reason for the disconnection.
*/
public DisconnectingEvent(Session session, Component reason, Throwable cause) {
public DisconnectingEvent(Session session, DisconnectionDetails disconnectionDetails) {
this.session = session;
this.reason = reason;
this.cause = cause;
this.disconnectionDetails = disconnectionDetails;
}

/**
Expand All @@ -38,17 +35,8 @@ public Session getSession() {
*
* @return The event's reason.
*/
public Component getReason() {
return this.reason;
}

/**
* Gets the Throwable responsible for the session disconnecting.
*
* @return The Throwable responsible for the disconnect, or null if the disconnect was not caused by a Throwable.
*/
public Throwable getCause() {
return this.cause;
public DisconnectionDetails getDetails() {
return this.disconnectionDetails;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,12 @@ public Session getSession() {
/**
* Gets the packet involved in this event as the required type.
*
* @param <T> Type of the packet.
* @return The event's packet as the required type.
* @throws IllegalStateException If the packet's value isn't of the required type.
*/
@SuppressWarnings("unchecked")
public <T extends Packet> T getPacket() {
public Packet getPacket() {
try {
return (T) this.packet;
return this.packet;
} catch (ClassCastException e) {
throw new IllegalStateException("Tried to get packet as the wrong type. Actual type: " + this.packet.getClass().getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import lombok.experimental.Accessors;
import org.geysermc.mcprotocollib.network.ProxyInfo;
import org.geysermc.mcprotocollib.network.netty.DefaultPacketHandlerExecutor;
import org.geysermc.mcprotocollib.network.packet.PacketProtocol;
import org.geysermc.mcprotocollib.network.session.ClientNetworkSession;
import org.geysermc.mcprotocollib.protocol.MinecraftProtocol;

import java.net.InetSocketAddress;
import java.net.SocketAddress;
Expand All @@ -18,7 +18,7 @@
@NoArgsConstructor(access = lombok.AccessLevel.PRIVATE)
public final class ClientNetworkSessionFactory {
private SocketAddress remoteSocketAddress;
private PacketProtocol protocol;
private MinecraftProtocol protocol;
private Executor packetHandlerExecutor;
private SocketAddress bindSocketAddress;
private ProxyInfo proxy;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.geysermc.mcprotocollib.network.helper;

import net.kyori.adventure.text.Component;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;

public record DisconnectionDetails(@NonNull Component reason, @Nullable Throwable cause) {
public DisconnectionDetails(@NonNull Component reason) {
this(reason, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,8 @@ public final void registerClientbound(PacketDefinition<? extends Packet> definit
* @return The created packet.
* @throws IllegalArgumentException If the packet ID is not registered.
*/
@SuppressWarnings("unchecked")
public Packet createClientboundPacket(int id, ByteBuf buf) {
PacketDefinition<?> definition = (PacketDefinition<?>) this.clientbound.get(id);
PacketDefinition<?> definition = this.clientbound.get(id);
if (definition == null) {
throw new IllegalArgumentException("Invalid packet id: " + id);
}
Expand Down Expand Up @@ -166,9 +165,8 @@ public Class<? extends Packet> getClientboundClass(int id) {
* @return The created packet.
* @throws IllegalArgumentException If the packet ID is not registered.
*/
@SuppressWarnings("unchecked")
public Packet createServerboundPacket(int id, ByteBuf buf) {
PacketDefinition<?> definition = (PacketDefinition<?>) this.serverbound.get(id);
PacketDefinition<?> definition = this.serverbound.get(id);
if (definition == null) {
throw new IllegalArgumentException("Invalid packet id: " + id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.geysermc.mcprotocollib.network.event.server.SessionAddedEvent;
import org.geysermc.mcprotocollib.network.event.server.SessionRemovedEvent;
import org.geysermc.mcprotocollib.network.packet.PacketProtocol;
import org.geysermc.mcprotocollib.protocol.MinecraftProtocol;

import java.net.SocketAddress;
import java.util.ArrayList;
Expand All @@ -23,14 +24,14 @@

public abstract class AbstractServer implements Server {
private final SocketAddress bindAddress;
private final Supplier<? extends PacketProtocol> protocolSupplier;
private final Supplier<? extends MinecraftProtocol> protocolSupplier;

private final List<Session> sessions = new ArrayList<>();

private final Map<String, Object> flags = new HashMap<>();
private final List<ServerListener> listeners = new ArrayList<>();

public AbstractServer(SocketAddress bindAddress, Supplier<? extends PacketProtocol> protocolSupplier) {
public AbstractServer(SocketAddress bindAddress, Supplier<? extends MinecraftProtocol> protocolSupplier) {
this.bindAddress = bindAddress;
this.protocolSupplier = protocolSupplier;
}
Expand All @@ -45,7 +46,7 @@ public Supplier<? extends PacketProtocol> getPacketProtocol() {
return this.protocolSupplier;
}

protected PacketProtocol createPacketProtocol() {
protected MinecraftProtocol createPacketProtocol() {
return this.protocolSupplier.get();
}

Expand Down
Loading