YiC200333 / XConomy

An economy plugin that supports data synchronization between multiple servers
GNU General Public License v3.0
92 stars 32 forks source link

Wrong usage of CompletableFuture causing the server to hang? #140

Open kolderz opened 1 month ago

kolderz commented 1 month ago

Version of XConomy
XConomy 2.25.15

Server version
Purpur 1.20.4

Storage-type
MySQL

Describe the issues

The method causing the server to time out might be related to issue #137

Maybe you should use CompletableFuture#get with a timeout parameter. But what's the point of using CompletableFuture when you block the main thread with the get method.

You're doing this a lot throughout your code, maybe consider your use of CompletableFuture.

https://github.com/YiC200333/XConomy/blob/master/XConomy-Bukkit/src/main/java/me/yic/xconomy/task/ReceivePerCheck.java#L33



[Watchdog Thread/ERROR]:    PID: 31 | Suspended: false | Native: false | State: WAITING
[Watchdog Thread/ERROR]:    Stack:
[Watchdog Thread/ERROR]:        java.base@17.0.11/jdk.internal.misc.Unsafe.park(Native Method)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.locks.LockSupport.park(LockSupport.java:211)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.CompletableFuture$Signaller.block(CompletableFuture.java:1864)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.ForkJoinPool.unmanagedBlock(ForkJoinPool.java:3465)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3436)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.CompletableFuture.waitingGet(CompletableFuture.java:1898)
[Watchdog Thread/ERROR]:        java.base@17.0.11/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:2072)
[Watchdog Thread/ERROR]:        XConomy-Paper-2.25.15.jar//me.yic.xconomy.task.ReceivePerCheck.hasreceivepermission(ReceivePerCheck.java:33)
[Watchdog Thread/ERROR]:        XConomy-Paper-2.25.15.jar//me.yic.xconomy.command.core.CommandPay.onCommand(CommandPay.java:113)
[Watchdog Thread/ERROR]:        XConomy-Paper-2.25.15.jar//me.yic.xconomy.command.core.CommandCore.onCommand(CommandCore.java:111)
[Watchdog Thread/ERROR]:        XConomy-Paper-2.25.15.jar//me.yic.xconomy.Commands.onCommand(Commands.java:33)
[Watchdog Thread/ERROR]:        org.bukkit.command.PluginCommand.execute(PluginCommand.java:45)```
YiC200333 commented 1 month ago

Thanks for your help, I will fix it.

YiC200333 commented 1 month ago

I have removed the CompletableFuture from the data query functionality in version 2.26.3. However, the detection of offline player permissions still requires the use of CompletableFuture. Otherwise, the following error may occur.

The lookup request was made on the main server thread. It is not safe to execute a request to load data for offline players from the database in this context. If you are a plugin author, please consider making your request asynchronously. Alternatively, server admins can disable this catch by setting 'vault-unsafe-lookups' to true in the LP config, but should consider the consequences (lag) before doing so.

I will add a new option to skip using CompletableFuture for servers that do not require offline player permission checks

kolderz commented 1 month ago

You can still use the CompletableFuture. I think you should change how you handle it.

Here's a naive example: The method that queries the user from the database, should return a CompletableFuture.

Then the method that calls payUser can use whenComplete to handle the error and retrieve the result.

payUser(playerId)
 .orTimeout(1, TimeUnit.SECONDS) // this might be pointless because HikariCP will throw exception if timeout occur
 .whenComplete((user, error) -> {
   if(error != null) {
      // .. assuming some SQLException occur
      // log something out
      player.sendMessage("Error while paying user");
      return;
   }

    if(user != null) {
      Bukkit.getScheduler().runTask(plugin, () -> {
            // run on main thread
      });
    }
 });
YiC200333 commented 1 month ago

Thank you for your help. The new version has added a timeout feature, and the time can be configured.