Skip to content

Commit

Permalink
pass toString() to logger call to avoid holding ConnectedPlayer object
Browse files Browse the repository at this point in the history
  • Loading branch information
mdxd44 committed Nov 28, 2024
1 parent 6fbb412 commit d123b13
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 35 deletions.
7 changes: 4 additions & 3 deletions .github/workflows/gradle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@ jobs:
- name: Cache Minecraft reports
uses: actions/cache@v4.1.2
with:
path: plugin/build/minecraft/*/ # Cache only the directories
key: minecraft-${{ hashFiles('gradle.properties') }} # Cache key based on gradle.properties because of gameVersions property
path: |
plugin/build/minecraft/
plugin/build/generated/minecraft/mappings/
key: minecraft-${{ hashFiles('gradle.properties') }} # Cache key is based on gradle.properties because of gameVersions property
- name: Setup Java
uses: actions/setup-java@v4.5.0
with:
distribution: temurin
java-version: 21
check-latest: true
cache: gradle
- name: Setup Gradle
uses: gradle/actions/setup-gradle@v3.3.2
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ jobs:
uses: actions/cache@v4.1.2
with:
path: |
plugin/build/minecraft/*/
plugin/build/generated/minecraft/mappings
key: minecraft-${{ hashFiles('gradle.properties') }} # Cache key based on gradle.properties because of gameVersions property
plugin/build/minecraft/
plugin/build/generated/minecraft/mappings/
key: minecraft-${{ hashFiles('gradle.properties') }} # Cache key is based on gradle.properties because of gameVersions property
- name: Setup Java
uses: actions/setup-java@v4.5.0
with:
distribution: temurin
java-version: 21
check-latest: true
cache: gradle
- name: Setup Gradle
uses: gradle/actions/setup-gradle@v3.3.2
Expand Down
6 changes: 4 additions & 2 deletions plugin/src/main/java/net/elytrium/limboapi/LimboAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,10 @@ public LimboAPI(Logger logger, ProxyServer server, Metrics.Factory metricsFactor
this.server.shutdown();
return;
} else if (maximumProtocolVersionNumber != SUPPORTED_MAXIMUM_PROTOCOL_VERSION_NUMBER) {
LOGGER.warn("Current LimboAPI version doesn't support current Velocity version (protocol version numbers: supported - {}, velocity - {})",
SUPPORTED_MAXIMUM_PROTOCOL_VERSION_NUMBER, maximumProtocolVersionNumber);
LOGGER.warn(
"Current LimboAPI version doesn't support current Velocity version (protocol version numbers: supported - {}, velocity - {})",
SUPPORTED_MAXIMUM_PROTOCOL_VERSION_NUMBER, maximumProtocolVersionNumber
);
LOGGER.warn("Please update LimboAPI (https://github.com/Elytrium/LimboAPI/releases). LimboAPI support: https://ely.su/discord");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ private void fireRegisterEvent(ConnectedPlayer player, MinecraftConnection conne
this.plugin.setKickCallback(player, limboRegisterEvent.getOnKickCallback());
queue.next();
}, connection.eventLoop()).exceptionally(t -> {
LimboAPI.getLogger().error("Exception while registering LimboAPI login handlers for {}.", player, t);
LimboAPI.getLogger().error("Exception while registering LimboAPI login handlers for {}", player.toString(), t);
return null;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public void next() {
}
}

@SuppressWarnings("UnnecessaryToStringCall")
private void finish() {
this.plugin.removeLoginQueue(this.player);

Expand Down Expand Up @@ -161,7 +162,7 @@ private void finish() {
try {
SET_PERMISSION_FUNCTION_METHOD.invokeExact(this.player, function);
} catch (Throwable t) {
logger.error("Exception while completing injection to {}", this.player, t);
logger.error("Exception while completing injection to {}", this.player.toString(), t);
}
}

Expand All @@ -173,12 +174,13 @@ private void finish() {
}
}, connection.eventLoop());
} catch (Throwable t) {
logger.error("Exception while completing injection to {}", this.player, t);
logger.error("Exception while completing injection to {}", this.player.toString(), t);
}
}, connection.eventLoop());
}

// From Velocity
@SuppressWarnings("UnnecessaryToStringCall")
@SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE")
private void initialize(MinecraftConnection connection) {
connection.setAssociation(this.player);
Expand All @@ -203,25 +205,23 @@ private void initialize(MinecraftConnection connection) {
Optional<Component> reason = event.getResult().getReasonComponent();
if (reason.isPresent()) {
this.player.disconnect0(reason.get(), false);
} else {
if (this.server.registerConnection(this.player)) {
if (connection.getActiveSessionHandler() instanceof LoginConfirmHandler confirm) {
confirm.waitForConfirmation(() -> this.connectToServer(connection));
} else {
this.connectToServer(connection);
}
} else if (this.server.registerConnection(this.player)) {
if (connection.getActiveSessionHandler() instanceof LoginConfirmHandler confirm) {
confirm.waitForConfirmation(() -> this.connectToServer(connection));
} else {
this.player.disconnect0(Component.translatable("velocity.error.already-connected-proxy"), false);
this.connectToServer(connection);
}
} else {
this.player.disconnect0(Component.translatable("velocity.error.already-connected-proxy"), false);
}
}
}, connection.eventLoop()).exceptionally(t -> {
logger.error("Exception while completing login initialisation phase for {}", this.player, t);
logger.error("Exception while completing login initialisation phase for {}", this.player.toString(), t);
return null;
});
}

@SuppressWarnings("unchecked")
@SuppressWarnings({"DataFlowIssue", "unchecked", "UnnecessaryToStringCall"})
@SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE")
private void connectToServer(MinecraftConnection connection) {
if (connection.getProtocolVersion().lessThan(ProtocolVersion.MINECRAFT_1_20_2)) {
Expand Down Expand Up @@ -267,7 +267,7 @@ private void connectToServer(MinecraftConnection connection) {
throw new ReflectionException(t);
}
}).exceptionally(t -> {
LimboAPI.getLogger().error("Exception while connecting {} to initial server", this.player, t);
LimboAPI.getLogger().error("Exception while connecting {} to initial server", this.player.toString(), t);
return null;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,13 @@ public CompletableFuture<Void> thenRun(Runnable runnable) {
return this.confirmation.thenRun(runnable);
}

@SuppressWarnings("UnnecessaryCallToStringValueOf")
public void waitForConfirmation(Runnable runnable) {
this.thenRun(() -> {
try {
runnable.run();
} catch (Throwable t) {
LimboAPI.getLogger().error("Failed to confirm transition for {}", this.player, t);
LimboAPI.getLogger().error("Failed to confirm transition for {}", this.player.toString(), t);
}

try {
Expand All @@ -72,13 +73,13 @@ public void waitForConfirmation(Runnable runnable) {
try {
this.connection.channelRead(ctx, packet);
} catch (Throwable t) {
LimboAPI.getLogger().error("{}: exception handling exception in {}", ctx.channel().remoteAddress(), this.connection.getActiveSessionHandler(), t);
LimboAPI.getLogger().error("{}: failed to handle packet {} for handler {}", this.player.toString(), packet.toString(), String.valueOf(this.connection.getActiveSessionHandler()), t);
}
}

this.queuedPackets.clear();
} catch (Throwable t) {
LimboAPI.getLogger().error("Failed to process packet queue for {}", this.player, t);
LimboAPI.getLogger().error("Failed to process packet queue for {}", this.player.toString(), t);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ protected void spawnPlayerLocal(Class<? extends LimboSessionHandler> handlerClas
connection.flush();
}

@SuppressWarnings("UnnecessaryToStringCall")
private void spawnPlayerLocal(ConnectedPlayer player, LimboSessionHandler handler, RegisteredServer previousServer) {
MinecraftConnection connection = player.getConnection();
connection.eventLoop().execute(() -> {
Expand All @@ -457,7 +458,7 @@ private void spawnPlayerLocal(ConnectedPlayer player, LimboSessionHandler handle
}

if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().info("{} ({}) has connected to the {} Limbo", player.getUsername(), player.getRemoteAddress(), this.limboName);
LimboAPI.getLogger().info("{} has connected to the {} Limbo", player.toString(), this.limboName);
}

// With an abnormally large number of connections from the same nickname,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,15 @@ public LimboSessionHandlerImpl(LimboAPI plugin, LimboImpl limbo, ConnectedPlayer
}
}

@SuppressWarnings("UnnecessaryToStringCall")
public void onConfig(LimboPlayer player) {
this.loaded = true;
this.limboPlayer = player;
this.callback.onConfig(this.limbo, player);

int serverReadTimeout = Objects.requireNonNullElseGet(this.limbo.getReadTimeout(), () -> this.plugin.getServer().getConfiguration().getReadTimeout());

// We should always send multiple keepalives inside a single timeout to not trigger Netty read timeout.
// We should always send multiple keepalives inside a single timeout to not trigger Netty read timeout
serverReadTimeout /= 2;

this.keepAliveTask = player.getScheduledExecutor().scheduleAtFixedRate(() -> {
Expand All @@ -135,9 +136,9 @@ public void onConfig(LimboPlayer player) {

if (this.keepAlivePending) {
if (++this.keepAlivesSkipped == 2) {
connection.closeWith(this.plugin.getPackets().getTimeOut(this.player.getConnection().getState()));
connection.closeWith(this.plugin.getPackets().getTimeOut(connection.getState()));
if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().warn("{} was kicked due to keepalive timeout.", this.player);
LimboAPI.getLogger().warn("{} was kicked due to keepalive timeout", this.player.toString());
}
}
} else if (this.keepAliveSentTime == 0 && this.originalHandler instanceof LimboSessionHandlerImpl sessionHandler) {
Expand Down Expand Up @@ -192,6 +193,7 @@ public void disconnectToConfig(Runnable runnable) {
}

@Override
@SuppressWarnings("UnnecessaryToStringCall")
public boolean handle(FinishedUpdatePacket packet) {
// Switching to CONFIG state
if (this.player.getConnection().getState() != StateRegistry.CONFIG) {
Expand All @@ -206,7 +208,7 @@ public boolean handle(FinishedUpdatePacket packet) {
this.player.getConnection().closeWith(this.plugin.getPackets().getInvalidSwitch());

if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().warn("{} sent an unexpected state switch confirmation.", this.player);
LimboAPI.getLogger().warn("{} sent an unexpected state switch confirmation", this.player.toString());
}
}

Expand Down Expand Up @@ -271,13 +273,14 @@ public boolean handle(AcceptTeleportationPacket packet) {
}

@Override
@SuppressWarnings("UnnecessaryToStringCall")
public boolean handle(KeepAlivePacket packet) {
MinecraftConnection connection = this.player.getConnection();
if (this.keepAlivePending) {
if (packet.getRandomId() != this.keepAliveKey) {
connection.closeWith(this.plugin.getPackets().getInvalidPing());
if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().warn("{} sent an invalid keepalive.", this.player);
LimboAPI.getLogger().warn("{} sent an invalid keepalive", this.player.toString());
}

return false;
Expand All @@ -291,7 +294,7 @@ public boolean handle(KeepAlivePacket packet) {
} else {
connection.closeWith(this.plugin.getPackets().getInvalidPing());
if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().warn("{} sent an unexpected keepalive.", this.player);
LimboAPI.getLogger().warn("{} sent an unexpected keepalive", this.player.toString());
}

return false;
Expand Down Expand Up @@ -381,10 +384,11 @@ public void handleGeneric(MinecraftPacket packet) {
this.callback.onGeneric(packet);
}

@SuppressWarnings("UnnecessaryToStringCall")
private void kickTooBigPacket(String type, int length) {
this.player.getConnection().closeWith(this.plugin.getPackets().getTooBigPacket());
if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().warn("{} sent too big packet. (type: {}, length: {})", this.player, type, length);
LimboAPI.getLogger().warn("{} sent too big packet (type: {}, length: {})", this.player.toString(), type, length);
}
}

Expand All @@ -409,7 +413,7 @@ public void disconnected() {
this.release();

if (Settings.IMP.MAIN.LOGGING_ENABLED) {
LimboAPI.getLogger().info("{} ({}) has disconnected from the {} Limbo", this.player.getUsername(), this.player.getRemoteAddress(), this.limbo.getName());
LimboAPI.getLogger().info("{} has disconnected from the {} Limbo", this.player.toString(), this.limbo.getName());
}

MinecraftConnection connection = this.player.getConnection();
Expand Down

0 comments on commit d123b13

Please sign in to comment.