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

MessageUtil.doDelayed/postTick is thread-unsafe #4526

Closed
Johni0702 opened this issue Feb 8, 2020 · 1 comment
Closed

MessageUtil.doDelayed/postTick is thread-unsafe #4526

Johni0702 opened this issue Feb 8, 2020 · 1 comment

Comments

@Johni0702
Copy link

@Johni0702 Johni0702 commented Feb 8, 2020

The MessageUtil.doDelayed system relies on a single backing queue:

private static final DelayedList<Runnable> DELAYED_TASKS = DelayedList.createConcurrent();
public static void doDelayed(Runnable task) {
doDelayed(1, task);
}
public static void doDelayed(int delay, Runnable task) {
DELAYED_TASKS.add(delay, task);
}
public static void postTick() {
for (Runnable runnable : DELAYED_TASKS.advance()) {
runnable.run();
}
}

It is however used on the client (ref) and the server (ref), leading to runnables being shuffled between both threads at random and all the fun stuff that follows from that (e.g. Johni0702/BetterPortals#458).

Example stack trace of illegal access (with modified version of BP)
[16:09:43] [Client thread/ERROR] [betterportals/view]: Exception caught in net handler of view of EntityPlayerMP['Johni0702'/207, l='#458', x=563.30, y=4.31, z=-423.70]: 
java.util.ConcurrentModificationException: channel is locked for iteration
	at de.johni0702.minecraft.view.impl.server.SafeEmbeddedChannel.handleOutboundMessage(ServerWorldsManagerImpl.kt:41) ~[SafeEmbeddedChannel.class:?]
	at io.netty.channel.embedded.EmbeddedChannel.doWrite(EmbeddedChannel.java:674) ~[EmbeddedChannel.class:4.1.9.Final]
	at io.netty.channel.AbstractChannel$AbstractUnsafe.flush0(AbstractChannel.java:856) ~[AbstractChannel$AbstractUnsafe.class:4.1.9.Final]
	at io.netty.channel.AbstractChannel$AbstractUnsafe.flush(AbstractChannel.java:823) ~[AbstractChannel$AbstractUnsafe.class:4.1.9.Final]
	at io.netty.channel.DefaultChannelPipeline$HeadContext.flush(DefaultChannelPipeline.java:1296) ~[DefaultChannelPipeline$HeadContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush0(AbstractChannelHandlerContext.java:776) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:768) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:749) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.ChannelOutboundHandlerAdapter.flush(ChannelOutboundHandlerAdapter.java:115) ~[ChannelOutboundHandlerAdapter.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush0(AbstractChannelHandlerContext.java:776) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:768) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:749) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.ChannelOutboundHandlerAdapter.flush(ChannelOutboundHandlerAdapter.java:115) ~[ChannelOutboundHandlerAdapter.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush0(AbstractChannelHandlerContext.java:776) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush(AbstractChannelHandlerContext.java:768) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.flush(AbstractChannelHandlerContext.java:749) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at net.minecraftforge.fml.common.network.handshake.NetworkDispatcher.flush(NetworkDispatcher.java:555) ~[NetworkDispatcher.class:?]
	at io.netty.channel.AbstractChannelHandlerContext.invokeFlush0(AbstractChannelHandlerContext.java:776) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:802) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:814) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:794) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:831) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.DefaultChannelPipeline.writeAndFlush(DefaultChannelPipeline.java:1032) ~[DefaultChannelPipeline.class:4.1.9.Final]
	at io.netty.channel.AbstractChannel.writeAndFlush(AbstractChannel.java:296) ~[AbstractChannel.class:4.1.9.Final]
	at net.minecraft.network.NetworkManager.func_150732_b(NetworkManager.java:225) ~[gw.class:?]
	at net.minecraft.network.NetworkManager.func_179290_a(NetworkManager.java:168) ~[gw.class:?]
	at net.minecraftforge.fml.common.network.handshake.NetworkDispatcher.sendProxy(NetworkDispatcher.java:462) ~[NetworkDispatcher.class:?]
	at net.minecraftforge.fml.common.network.FMLOutboundHandler.write(FMLOutboundHandler.java:389) ~[FMLOutboundHandler.class:?]
	at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:738) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeWrite(AbstractChannelHandlerContext.java:730) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:816) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:723) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.handler.codec.MessageToMessageEncoder.write(MessageToMessageEncoder.java:111) ~[MessageToMessageEncoder.class:4.1.9.Final]
	at io.netty.handler.codec.MessageToMessageCodec.write(MessageToMessageCodec.java:116) ~[MessageToMessageCodec.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:738) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:801) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:814) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:794) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:831) ~[AbstractChannelHandlerContext.class:4.1.9.Final]
	at io.netty.channel.DefaultChannelPipeline.writeAndFlush(DefaultChannelPipeline.java:1032) ~[DefaultChannelPipeline.class:4.1.9.Final]
	at io.netty.channel.AbstractChannel.writeAndFlush(AbstractChannel.java:296) ~[AbstractChannel.class:4.1.9.Final]
	at net.minecraftforge.fml.common.network.simpleimpl.SimpleNetworkWrapper.sendTo(SimpleNetworkWrapper.java:250) ~[SimpleNetworkWrapper.class:?]
	at buildcraft.lib.net.MessageManager.sendTo(MessageManager.java:221) ~[MessageManager.class:?]
	at buildcraft.core.BCCoreEventDist.lambda$onEntityJoinWorld$0(BCCoreEventDist.java:37) ~[BCCoreEventDist.class:?]
	at buildcraft.lib.misc.MessageUtil.postTick(MessageUtil.java:57) [MessageUtil.class:?]
	at buildcraft.lib.BCLibEventDist.clientTick(BCLibEventDist.java:141) [BCLibEventDist.class:?]
	at net.minecraftforge.fml.common.eventhandler.ASMEventHandler_27_BCLibEventDist_clientTick_ClientTickEvent.invoke(.dynamic) [?:?]
	at net.minecraftforge.fml.common.eventhandler.ASMEventHandler.invoke(ASMEventHandler.java:90) [ASMEventHandler.class:?]
	at net.minecraftforge.fml.common.eventhandler.EventBus.post(EventBus.java:182) [EventBus.class:?]
	at net.minecraftforge.fml.common.FMLCommonHandler.onPostClientTick(FMLCommonHandler.java:349) [FMLCommonHandler.class:?]
	at net.minecraft.client.Minecraft.func_71407_l(Minecraft.java:1911) [bib.class:?]
	at net.minecraft.client.Minecraft.func_71411_J(Minecraft.java:1098) [bib.class:?]
	at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:398) [bib.class:?]
	at net.minecraft.client.main.Main.main(SourceFile:123) [Main.class:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_242]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_242]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_242]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_242]
	at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) [launchwrapper-1.12.jar:?]
	at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_242]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_242]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_242]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_242]
	at org.multimc.onesix.OneSixLauncher.launchWithMainClass(OneSixLauncher.java:196) [NewLaunch.jar:?]
	at org.multimc.onesix.OneSixLauncher.launch(OneSixLauncher.java:231) [NewLaunch.jar:?]
	at org.multimc.EntryPoint.listen(EntryPoint.java:143) [NewLaunch.jar:?]
	at org.multimc.EntryPoint.main(EntryPoint.java:34) [NewLaunch.jar:?]
@AlexIIL

This comment has been minimized.

Copy link
Member

@AlexIIL AlexIIL commented Feb 23, 2020

Ok, this should be fixed in 7.99.24.5 - please let me know if that's not the case.

@AlexIIL AlexIIL closed this Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.