search

elBukkit / PreciousStones

Self-service protection for Minecraft servers
http://dev.bukkit.org/server-mods/preciousstones/
10 stars 10 forks source link

/ps allowall is a blocking command #108

Open FireController1847 opened 3 years ago

FireController1847 commented 3 years ago

If a user has quite a few fields, it is very easy to crash a server by overloading the data system. I don't know if internally there is a cache or not, but when a user runs /ps allowall when they have quite a few fields, the server will freeze until all of the fields have been updated. If the server freezes for too long, the server will crash.

Is /ps allowall a blocking command internally, or is it multithreaded? If it's blocking, this is a feature request to make /ps allowall and /ps removeall a multithreaded command so it does not block the primary server thread.

xtomyserrax commented 3 years ago

This probably might be an issue, I dont know how the plugin is handling this stuff but I think its just blocking primary server thread. I saw recently the same issue with /ps locations radius or a similar command in which you set a radius amount and it will find all fields near. If the radius is big enough then server will crash.

Solutions could be done to make this methods async, add some limits, or make it multithreaded. Im not an expert though and this is speculation, I havent checked that code

FireController1847 commented 3 years ago

Can confirm this is a blocking command. My production server did a hard-crash and the last command ran was /ps allowall <player> coowner. Consecutive errors are that the server thread is taking too long to tick and ultimately crashed the server

xtomyserrax commented 3 years ago

I guess the best solution would be to make the method async (and also other methods such as /ps locations radius)

FireController1847 commented 3 years ago

UPDATE: I've found the command works fine when running with players, but when attempting to allowall a clan the server crashes. I am currently investigating this and will make a separate issue because this is also an issue, but not as much of a major one (it could be multithreaded but wasn't causing the crash)

xtomyserrax commented 3 years ago

The server crashes because probably it is taking a lot of time to get the lands and add that new information. Maybe doing this async will fix the issue but some checkers should be added to avoid collisions.