Skip to content

Commit a3dbe3d

Browse files
committed
Summarize packet error into periodic debug output; use more efficient data parsing.
1 parent 157f5bb commit a3dbe3d

File tree

13 files changed

+158
-37
lines changed

13 files changed

+158
-37
lines changed

arclight-common/src/main/java/io/izzel/arclight/common/bridge/bukkit/MessengerBridge.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package io.izzel.arclight.common.bridge.bukkit;
22

33
import io.izzel.arclight.common.mod.plugin.messaging.ArclightPluginChannel;
4+
import io.izzel.arclight.common.mod.plugin.messaging.PacketRecorder;
45
import it.unimi.dsi.fastutil.objects.Object2BooleanOpenHashMap;
56
import net.minecraft.resources.ResourceLocation;
67
import org.bukkit.craftbukkit.v.entity.CraftPlayer;
@@ -19,4 +20,6 @@ public interface MessengerBridge {
1920
void bridge$registerAnonymousOutgoing(ResourceLocation location);
2021
ArclightPluginChannel<?> bridge$getAndCheckCrossSend(Plugin src, ResourceLocation channel);
2122
void bridge$checkUnsafeSend(Plugin src, ResourceLocation channel);
23+
24+
PacketRecorder bridge$getPacketRecorder();
2225
}

arclight-common/src/main/java/io/izzel/arclight/common/bridge/core/network/login/ServerLoginPacketListenerBridge.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ public interface ServerLoginPacketListenerBridge {
1717
void bridge$disconnect(String reason);
1818

1919
default FriendlyByteBuf bridge$getDiscardedQueryAnswerData(ServerboundCustomQueryAnswerPacket packet) {
20-
return packet.payload() instanceof ArclightCustomQueryAnswerPayload payload
21-
? new FriendlyByteBuf(payload.buf())
20+
return packet.payload() instanceof ArclightCustomQueryAnswerPayload(io.netty.buffer.ByteBuf buf)
21+
? new FriendlyByteBuf(buf)
2222
: null;
2323
}
2424

arclight-common/src/main/java/io/izzel/arclight/common/mixin/bukkit/StandardMessengerMixin.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.google.common.collect.MultimapBuilder;
44
import com.google.common.collect.SetMultimap;
55
import io.izzel.arclight.common.bridge.bukkit.MessengerBridge;
6+
import io.izzel.arclight.common.mod.plugin.messaging.PacketRecorder;
67
import io.izzel.arclight.common.mod.server.ArclightServer;
78
import io.izzel.arclight.common.mod.plugin.messaging.ArclightPluginChannel;
89
import net.minecraft.resources.ResourceLocation;
@@ -34,6 +35,9 @@ public abstract class StandardMessengerMixin implements Messenger, MessengerBrid
3435
@Unique
3536
private SetMultimap<Plugin, ResourceLocation> arclight$crossSend;
3637

38+
@Unique
39+
private PacketRecorder arclight$recorder;
40+
3741
@ModifyConstant(
3842
method = "validateAndCorrectChannel",
3943
constant = @Constant(intValue = Messenger.MAX_CHANNEL_SIZE)
@@ -88,6 +92,11 @@ public abstract class StandardMessengerMixin implements Messenger, MessengerBrid
8892
arclight$updateChannel(location, true);
8993
}
9094

95+
@Override
96+
public PacketRecorder bridge$getPacketRecorder() {
97+
return arclight$recorder;
98+
}
99+
91100
@Unique
92101
private void arclight$updateChannel(ResourceLocation location, boolean create) {
93102
if (location != null) {
@@ -115,6 +124,7 @@ public abstract class StandardMessengerMixin implements Messenger, MessengerBrid
115124
private void arclight$init(CallbackInfo ci) {
116125
arclight$registry = new HashMap<>();
117126
arclight$crossSend = MultimapBuilder.hashKeys().hashSetValues().build();
127+
arclight$recorder = new PacketRecorder();
118128
}
119129

120130
@Redirect(method = {"removeFromOutgoing*", "removeFromIncoming*"}, at = @At(value = "INVOKE", target = "Ljava/util/Map;remove(Ljava/lang/Object;)Ljava/lang/Object;"))
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package io.izzel.arclight.common.mod.plugin.messaging;
2+
3+
import io.izzel.arclight.common.mod.server.ArclightServer;
4+
import it.unimi.dsi.fastutil.objects.Object2IntArrayMap;
5+
import net.minecraft.Util;
6+
7+
import java.util.stream.Collectors;
8+
9+
public class PacketRecorder {
10+
private final Object2IntArrayMap<String> unknown = new Object2IntArrayMap<>();
11+
private long lastUpdate = Util.getMillis();
12+
13+
public PacketRecorder() {
14+
unknown.defaultReturnValue(0);
15+
}
16+
17+
public void recordUnknown(String id) {
18+
int num = unknown.getInt(id);
19+
unknown.put(id, num + 1);
20+
}
21+
22+
public void update() {
23+
long now = Util.getMillis();
24+
if (now - lastUpdate > 5*60*1000) {
25+
consumeAndLog();
26+
lastUpdate = now;
27+
}
28+
}
29+
30+
public void consumeAndLog() {
31+
String unknowns = unknown.object2IntEntrySet().stream()
32+
.map(it -> it.getKey() + '(' + it.getIntValue() + ')')
33+
.collect(Collectors.joining(", ", "unknown=[", "];"));
34+
unknown.clear();
35+
36+
ArclightServer.LOGGER.debug("Packet error statistics: {}", unknowns);
37+
}
38+
}

arclight-common/src/main/java/io/izzel/arclight/common/mod/plugin/messaging/RawPayload.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import com.google.common.base.Preconditions;
44
import io.netty.buffer.ByteBuf;
5-
import io.netty.buffer.Unpooled;
65
import net.minecraft.network.FriendlyByteBuf;
76
import net.minecraft.network.codec.StreamCodec;
87
import net.minecraft.network.protocol.common.custom.CustomPacketPayload;
@@ -18,9 +17,7 @@ static <B extends FriendlyByteBuf> StreamCodec<B, ArclightRawPayload> channelCod
1817
StreamCodec.of(FriendlyByteBuf::writeBytes, buf -> {
1918
var size = buf.readableBytes();
2019
Preconditions.checkArgument(size <= max, "Custom payload size may not be larger than " + max);
21-
ByteBuf data = Unpooled.buffer(size, size);
22-
buf.readBytes(data);
23-
return data;
20+
return buf.copy();
2421
}),
2522
RawPayload::data,
2623
it -> new ArclightRawPayload(type, it)
@@ -33,7 +30,7 @@ static <B extends FriendlyByteBuf> StreamCodec<B, CustomPacketPayload> discarded
3330
public DiscardedPayload decode(B buf) {
3431
int j = buf.readableBytes();
3532
if (j >= 0 && j <= max) {
36-
var heap = buf.readBytes(j);
33+
var heap = buf.copy();
3734
var payload = new DiscardedPayload(location);
3835
((RawPayload)(Object)payload).setData(heap);
3936
return payload;
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,22 @@
11
package io.izzel.arclight.fabric.mixin.core.network;
22

3+
import io.izzel.arclight.common.bridge.bukkit.MessengerBridge;
34
import io.izzel.arclight.common.bridge.core.network.common.ServerCommonPacketListenerBridge;
5+
import net.minecraft.network.protocol.common.ServerboundCustomPayloadPacket;
46
import net.minecraft.server.network.ServerCommonPacketListenerImpl;
7+
import org.bukkit.Bukkit;
58
import org.spongepowered.asm.mixin.Mixin;
9+
import org.spongepowered.asm.mixin.injection.At;
10+
import org.spongepowered.asm.mixin.injection.Inject;
11+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
612

713
@Mixin(ServerCommonPacketListenerImpl.class)
8-
public abstract class ServerCommonPacketListenerImplMixin_Fabric implements ServerCommonPacketListenerBridge {}
14+
public abstract class ServerCommonPacketListenerImplMixin_Fabric implements ServerCommonPacketListenerBridge {
15+
16+
@Inject(method = "handleCustomPayload", at = @At("TAIL"))
17+
private void arclight$handleUnknownPayload(ServerboundCustomPayloadPacket packet, CallbackInfo ci) {
18+
var recorder = ((MessengerBridge) Bukkit.getMessenger()).bridge$getPacketRecorder();
19+
recorder.recordUnknown(packet.payload().type().id().toString());
20+
recorder.update();
21+
}
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package io.izzel.arclight.fabric.mixin.core.network;
2+
3+
import io.izzel.arclight.common.bridge.bukkit.MessengerBridge;
4+
import net.minecraft.network.protocol.common.ServerboundCustomPayloadPacket;
5+
import net.minecraft.server.network.ServerGamePacketListenerImpl;
6+
import org.bukkit.Bukkit;
7+
import org.spongepowered.asm.mixin.Mixin;
8+
import org.spongepowered.asm.mixin.injection.At;
9+
import org.spongepowered.asm.mixin.injection.Inject;
10+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
11+
12+
@Mixin(ServerGamePacketListenerImpl.class)
13+
public class ServerGamePacketListenerImplMixin_Fabric {
14+
15+
@Inject(method = "handleCustomPayload", at = @At("TAIL"))
16+
private void arclight$recordUnknown(ServerboundCustomPayloadPacket packet, CallbackInfo ci) {
17+
var recorder = ((MessengerBridge) Bukkit.getMessenger()).bridge$getPacketRecorder();
18+
recorder.recordUnknown(packet.payload().type().id().toString());
19+
recorder.update();
20+
}
21+
}

arclight-forge/src/main/java/io/izzel/arclight/forge/mixin/forge/ForgeHooksMixin.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package io.izzel.arclight.forge.mixin.forge;
22

3+
import io.izzel.arclight.common.bridge.bukkit.MessengerBridge;
34
import io.izzel.arclight.common.mod.util.ArclightCaptures;
45
import net.minecraft.world.InteractionHand;
56
import net.minecraft.world.InteractionResult;
67
import net.minecraft.world.item.context.UseOnContext;
78
import net.minecraftforge.common.ForgeHooks;
9+
import net.minecraftforge.event.network.CustomPayloadEvent;
10+
import org.bukkit.Bukkit;
811
import org.spongepowered.asm.mixin.Mixin;
912
import org.spongepowered.asm.mixin.injection.At;
1013
import org.spongepowered.asm.mixin.injection.Inject;
@@ -22,4 +25,13 @@ public class ForgeHooksMixin {
2225
private static void arclight$removeHand(UseOnContext context, CallbackInfoReturnable<InteractionResult> cir) {
2326
ArclightCaptures.getPlaceEventHand(InteractionHand.MAIN_HAND);
2427
}
28+
29+
@Inject(method = "onCustomPayload(Lnet/minecraftforge/event/network/CustomPayloadEvent;)Z", at = @At("RETURN"))
30+
private static void arclight$recordUnknown(CustomPayloadEvent event, CallbackInfoReturnable<Boolean> cir) {
31+
if (!cir.getReturnValueZ()) {
32+
var recorder = ((MessengerBridge) Bukkit.getMessenger()).bridge$getPacketRecorder();
33+
recorder.recordUnknown(event.getChannel().toString());
34+
recorder.update();
35+
}
36+
}
2537
}

arclight-forge/src/main/java/io/izzel/arclight/forge/mod/plugin/messaging/ArclightForgeMessaging.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ public static ArclightPluginChannel<? extends ForgePayloadHandler> setupChannel(
2525
var registration = channel.getChannelHandler();
2626
try {
2727
NetworkRegistryAccessor.setLock(false);
28-
var simple = ChannelBuilder.named(location)
28+
var event = ChannelBuilder.named(location)
2929
.serverAcceptedVersions(ACCEPT_ALL)
3030
.optional()
31-
.simpleChannel();
32-
registration.initialize(simple);
31+
.eventNetworkChannel();
32+
registration.initialize(event);
3333
} finally {
3434
NetworkRegistryAccessor.setLock(true);
3535
}

arclight-forge/src/main/java/io/izzel/arclight/forge/mod/plugin/messaging/ArclightForgePayloadDestroyer.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import net.minecraftforge.event.network.CustomPayloadEvent;
77

88
public record ArclightForgePayloadDestroyer(ArclightPluginChannel<ArclightForgePayloadDestroyer> channel) implements ForgePayloadHandler, PayloadDestroyer {
9+
910
@Override
10-
public void accept(ArclightRawPayload arclightRawPayload, CustomPayloadEvent.Context context) {}
11+
public void accept(CustomPayloadEvent customPayloadEvent) {}
1112
}

0 commit comments

Comments
 (0)