hackclub / HCCore

🔌 Main plugin for the Hack Club Minecraft server
https://mc.hackclub.com
MIT License
26 stars 20 forks source link

NullPointerException in AFKListener on reload #77

Closed ifvictr closed 4 years ago

ifvictr commented 4 years ago

PlayerMoveEvent and PlayerInteractEvent have been observed throwing NPE errors on reload so far.

nice6599 commented 4 years ago

Can you add steps to reproduce this error? when i run /reload I don't run into any errors

ifvictr commented 4 years ago

I haven't really dug into it, but it seems like the errors are thrown when players move/interact and their PlayerData sessions haven't been initialized yet.

nice6599 commented 4 years ago

okay i did some basic testing and it seems like the error is caused by .setLastActiveAt() if you remove that there are no problems. if you remove System.currentTimeMillis() and replace it with an integer you still run into problems.

nice6599 commented 4 years ago

Initial analysis

Weirdly when you type into chat while the server is reloading you trigger two errors one in AFKListener.java and another in PlayerListener.java

in AFKListener.java it seems that the error is triggered by .setLastActiveAt() whereas in PlayerListener.java it seems to be triggered by the data.getMessageColor() both of these methods are found in PlayerData.java which says to me that that file is where the problem stems from.

You can trigger this same error by doing any of the following during reload

ifvictr commented 4 years ago

It's also worth noting that Spigot recommends doing a full restart instead of /reload.

nice6599 commented 4 years ago

is there any way to fix this? or are we just gonna have to deal with it?

hyperupcall commented 4 years ago

I replicated this too - seems like there is no way to fix it. i am also getting many java.lang.NoClassDefFoundErrors as well on reload most of the time hot reloading code has idiosyncrasies and can cause buggy behavior, especially if the implementation isn't officially supported by the whole toolchain (and it's no different) in the case of Spigot/Paper

so i'm closing this now because it only is activated when using Bile under the specific circumstances @nice6599 mentioned and even if we could theoretically fix it here, it would only make the codebase more cryptic or hard to keep making fixes. there doesn't seem to be an easy fix...

zanedb commented 4 years ago

I think the solution is to use reload as infrequently as possible, it's really not great for a bunch of reasons (this being one). Plugins like ServerUtils that can reload individual plugins are preferred.