Race condition in EnumWrappers#initialize

Question

Race condition in EnumWrappers#initialize

diogotcorreia opened this issue 5 months ago · 0 comments

diogotcorreia commented 5 months ago

This issue is not solved in a development build

Describe the bug
I'm using an async listener in my plugin, so it frequently happens that two packets that trigger initialization of EnumWrapper are sent at the same time.

However, due to the design of the EnumWrapper#initialize method, in an async context it is possible that the method returns without all fields having been initialized:

private static void initialize() {
    if (INITIALIZED)
        return; // (1)

    INITIALIZED = true; // (2)

    PROTOCOL_CLASS = MinecraftReflection.getEnumProtocolClass();
    // ...
    // (3)
}

If another thread calls initialize after the first thread reaches (2) but not (3), it will return immediately (1).

This error is a side effect of this issue (only happens once, when the first player joins the server):

[12:45:42 ERROR]: [Triton] Unhandled exception occurred in onAsyncPacket() for Triton
java.lang.NullPointerException: Cannot invoke "java.lang.Enum.name()" because "generic" is null
        at ProtocolLib.jar/com.comphenix.protocol.wrappers.EnumWrappers$EnumConverter.getSpecific(EnumWrappers.java:1104) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.wrappers.EnumWrappers$EnumConverter.getSpecific(EnumWrappers.java:1090) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.wrappers.WrappedTeamParameters.getColor(WrappedTeamParameters.java:73) ~[ProtocolLib.jar:?]
        at Triton.jar/com.rexcantor64.triton.packetinterceptor.ProtocolLibListener.handleScoreboardTeam(ProtocolLibListener.java:614) ~[Triton.jar:?]
        at Triton.jar/com.rexcantor64.triton.packetinterceptor.ProtocolLibListener.onPacketSending(ProtocolLibListener.java:755) ~[Triton.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler.lambda$processPacket$4(AsyncListenerHandler.java:602) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.timing.TimingTracker.lambda$static$0(TimingTracker.java:7) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler.processPacket(AsyncListenerHandler.java:600) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler.listenerLoop(AsyncListenerHandler.java:572) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler.access$100(AsyncListenerHandler.java:48) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler$1.run(AsyncListenerHandler.java:217) ~[ProtocolLib.jar:?]
        at ProtocolLib.jar/com.comphenix.protocol.async.AsyncListenerHandler.lambda$start$1(AsyncListenerHandler.java:286) ~[ProtocolLib.jar:?]
        at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:101) ~[paper-1.21.jar:1.21-68-8b35adc]
        at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:57) ~[paper-1.21.jar:1.21-68-8b35adc]
        at com.destroystokyo.paper.ServerSchedulerReportingWrapper.run(ServerSchedulerReportingWrapper.java:22) ~[paper-1.21.jar:?]
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
        at java.base/java.lang.Thread.run(Thread.java:1583) ~[?:?]

To Reproduce

(I'm not providing detailed steps since I'm going to be submitting a PR in a sec)

Setup Triton and TAB plugins in a server (e.g. Paper 1.21)
Join the server
Error on console (might only happen sometimes; it's a race condition after all)

Expected behavior
EnumWrappers#initialize should be thread-safe

Version Info
Built from commit 4aa344b

Additional context
Submitting a PR in a sec

Extra log where I added some print statements for extra clarity of bug:

[12:52:23 INFO]: [ProtocolLib] [STDOUT] INITIALIZED = true
[12:52:23 INFO]: [ProtocolLib] [STDOUT] CHAT_FORMATTING_CLASS = null
[12:52:23 INFO]: [ProtocolLib] [STDOUT] INITIALIZED = true
[12:52:23 INFO]: [ProtocolLib] [STDOUT] CHAT_FORMATTING_CLASS = null
[12:52:23 ERROR]: [Triton] Unhandled exception occurred in onAsyncPacket() for Triton
java.lang.NullPointerException: Cannot invoke "java.lang.Enum.name()" because "generic" is null
// -snip- same error as above
[12:52:23 INFO]: [Triton] [TRACE] Trying to get translation with key 'chat.test.0' in language 'en_GB'
[12:52:23 INFO]: [Triton] [TRACE] Found translation with key 'chat.test.0' in language 'en_GB'
[12:52:23 INFO]: [ProtocolLib] [STDOUT] INITIALIZED = true
[12:52:23 INFO]: [ProtocolLib] [STDOUT] CHAT_FORMATTING_CLASS = class net.minecraft.ChatFormatting
[12:52:23 INFO]: [ProtocolLib] [STDOUT] INITIALIZED = true
[12:52:23 INFO]: [ProtocolLib] [STDOUT] CHAT_FORMATTING_CLASS = class net.minecraft.ChatFormatting

Share to