EssentialsX

EssentialsX

2M Downloads

Calling ServerListPingEvent from other plugins causing UnsupportedOperationException

montlikadani opened this issue ยท 6 comments

commented

Information

Full output of /ess version:

Server version: 1.15.1-R0.1-SNAPSHOT git-Paper-50 (MC: 1.15.1)
EssentialsX version: 2.17.1.52
LuckPerms version: 5.0.57
Vault version: 1.7.3-b${env.TRAVIS_BUILD_NUMBER}
EssentialsXChat version: 2.17.1.52

Server log:
https://gist.github.com/montlikadani/a9b1128e647a4c5f5e238442e73c015a

Including: https://gist.github.com/montlikadani/69d2124861ada7903e99584ebf078bed
In this line: EssentialsServerListener.java#L73-L86
This is my code: PingScheduler.java#L42-L55

EssentialsX config:
https://gist.github.com/montlikadani/78b989d3899e60375809d0aa854b1b1c

Details

Description

As mentioned in the title, there is an older problem where I call ServerListPingEvent in the plugin code to ping the server and get some methods to connect to the server, if available. This has been fixed in the past, but not completely.

Steps to reproduce

Install EssentialsX to your server, create another plugin, call the ServerListPingEvent, complete the arguments to the method and call with or without asynchronously.

Example:

ServerListPingEvent ping = new ServerListPingEvent(
        new InetSocketAddress(/**Server host address converted to string*/,
       /**Server port*/).getAddress(), Bukkit.getMotd(),
       Bukkit.getOnlinePlayers().size(), Bukkit.getMaxPlayers());

Bukkit.getScheduler().runTaskAsynchronously(plugin,
       () -> Bukkit.getPluginManager().callEvent(ping));

Same issues/PRs: #868, #2381, #1442, #1419 #1436, #2381

commented

With this Spigot commit changes, still throws the error.

commented

I don't think there should be a reflection to delete an item from the list because with or without it the same error will occur as we try to remove an item from the list using Iterator. To prevent this, we could do not use ServerListPingEvent but try to use reflection to remove the vanish players from the tablist, thus making the plugin more secure for other plugins. I know that reflection is slow and harder to understand, but a better solution would be to correct such cases.

commented

Either implement the event fully according to the API so that plugins will support your plugin properly, or don't use the event at all. You can almost always achieve what you need without using that event. Trying to fix this on EssentialsX would require inefficient slow reflection every single time we try and handle the event, which we're not going to do. edit: This is a different but related issue to the one I was initially thinking of, but we're not going to suppress this exception unless there is a very valid reason to do so.

commented

Reflection is necessary to determine whether we need to use the event methods from Paper or those from Bukkit. This works reliably except for when plugins don't implement the actual API methods.

Altering the tab list is irrelevant; neither Minecraft nor CB/Spigot/Paper use the tab list for setting the player list.

commented

So the only solution is to use the Bukkit#isPrimaryThread method to fix the bug?

commented

That has nothing to do with it.

Either implement ServerListPingEvent and include proper implementations of the methods from the event, or use an alternative method to find whatever information you're trying to get.