GlowstoneMC / Glowstone

A fast, customizable and compatible open source server for Minecraft: Java Edition
https://glowstone.net
Other
1.89k stars 271 forks source link

Remapping dependencies #190

Closed Paulomart closed 8 years ago

Paulomart commented 8 years ago

When connecting to a glowstone server via bungee this happens and you cannot connect to the server.

15:17:19 [SEVERE] Error in network input
io.netty.handler.codec.DecoderException: java.lang.IndexOutOfBoundsException: readerIndex(3) + length(80) exceeds writerIndex(11): UnpooledUnsafeDirectByteBuf(ridx: 3, widx: 11, cap: 11)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:99)
at io.netty.handler.codec.MessageToMessageCodec.channelRead(MessageToMessageCodec.java:111)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:333)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:319)
at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:163)
at io.netty.handler.codec.ByteToMessageCodec.channelRead(ByteToMessageCodec.java:108)
at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:333)
at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:319)
at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:787)
at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:125)
at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:511)
at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:468)
at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:382)
at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:354)
at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:116)
at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IndexOutOfBoundsException: readerIndex(3) + length(80) exceeds writerIndex(11): UnpooledUnsafeDirectByteBuf(ridx: 3, widx: 11, cap: 11)
at io.netty.buffer.AbstractByteBuf.checkReadableBytes(AbstractByteBuf.java:1171)
at io.netty.buffer.AbstractByteBuf.readBytes(AbstractByteBuf.java:677)
at io.netty.buffer.AbstractByteBuf.readBytes(AbstractByteBuf.java:685)
at com.flowpowered.networking.util.ByteBufUtils.readUTF8(ByteBufUtils.java:46)
at net.glowstone.net.codec.handshake.HandshakeCodec.decode(HandshakeCodec.java:14)
at net.glowstone.net.codec.handshake.HandshakeCodec.decode(HandshakeCodec.java:10)
at net.glowstone.net.pipeline.CodecsHandler.decode(CodecsHandler.java:53)
at net.glowstone.net.pipeline.CodecsHandler.decode(CodecsHandler.java:19)
at io.netty.handler.codec.MessageToMessageCodec$2.decode(MessageToMessageCodec.java:81)
at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:89)
... 16 more

Related:

https://github.com/GlowstonePlusPlus/GlowstonePlusPlus/blob/master/src/main/java/net/glowstone/net/codec/handshake/HandshakeCodec.java

https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/ServerConnector.java#L76

https://github.com/SpigotMC/BungeeCord/blob/master/protocol/src/main/java/net/md_5/bungee/protocol/packet/Handshake.java

Paulomart commented 8 years ago

Somehow related to: SpigotMC/BungeeCord#1717

Paulomart commented 8 years ago

Also: I is not happing on my local machine, just in production

gdude2002 commented 8 years ago

According to this code, we do support LilyPad and Bungee. Perhaps @mastercoms could take a look at this, or maybe @turt2live if he hasn't dropped off the face of the planet, since he seems to know how it works.

Paulomart commented 8 years ago

I found the issue: I shade an older version of flowpowered into my plugins, it caused the throuble. Updating to 1.1.0 from glowstone++ fixed it.

gdude2002 commented 8 years ago

@Paulomart For future reference, you should remap everything you shade into your plugins to avoid conflicts like this. Here's how you do that with Maven - A quick Google didn't find anything for Gradle, however.

deathcap commented 8 years ago

@gdude2002 @Paulomart for Gradle, the maven-shade-plugin equivalent is shadow (also supports class relocation to avoid these conflicts)

gdude2002 commented 8 years ago

@deathcap Ah, that's what I use, didn't realise it was able to remap as well. Thanks.

Paulomart commented 8 years ago

Year I know that, but I application wraps around glowstone and injects libs before glowstone itself starts. Remapping the whole lib would cause a lot of problems in other projects that uses some parts of it as api. So I will go with using glowstones flowpowered to avoid this.

Paulomart commented 8 years ago

If anyone is interested I remapped and patched Glowstone: https://github.com/Mystalion/MysticStone

deathcap commented 8 years ago

Glowstone should probably take this change: https://github.com/Mystalion/MysticStone/blob/master/patches/0001-Pom-changes.patch

+                            <relocations>
+                               <relocation>
+                                    <pattern>com.flowpowered</pattern>
+                                    <shadedPattern>net.glowstone.lib.com.flowpowered</shadedPattern>
+                                </relocation>
+                                <relocation>
+                                    <pattern>io.netty</pattern>
+                                    <shadedPattern>net.glowstone.lib.io.netty</shadedPattern>
+                                </relocation>
+                            </relocations>

@gdude2002 @mastercoms what do you think? While plugins should ideally shade and relocate their own dependencies, Glowstone could help prevent conflicts by relocating its dependencies as well (similar to CraftBukkit of yore relocating its library dependencies into org.craftbukkit.libs.*)

Paulomart commented 8 years ago

When doing that, glowstone should divide in libs that are internal such as Netty and FlowPowered, and something like Apache commons which is often used in plugins without shading it.

gdude2002 commented 8 years ago

@deathcap It's a good idea, honestly. Personally I would say that plugins shouldn't be relying on internal libs for Glowstone, but as @Paulomart says, we should consider not remapping things that are in Bukkit and that plugins typically rely on (such as the MySQL driver).

kamcio96 commented 8 years ago

https://github.com/GlowstonePlusPlus/Glowkit/blob/bukkit%2Bglowkit/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java#L97 Here, the order should be changed.