BungeePerms icon indicating copy to clipboard operation
BungeePerms copied to clipboard

[Bug]: BungeePermsAPI not thread safe

Open Cheos137 opened this issue 2 years ago • 1 comments

Describe the bug BungeePerms internals seem to be not thread-safe, causing issues mostly (but not limited to) Velocity proxies due to the asynchronous nature of certain systems.

To Reproduce

  1. within a sample velocity sample plugin, create a join event listener calling into the bungeeperms api, in this case, specifically adding groups to a player.
  2. observe the api call resulting in a concurrency error within bungeeperms internals

Expected behavior The api call does not throw a concurrency-related exception

Screenshots or log

java.util.concurrent.CompletionException: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException
        at java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:315) ~[?:?]
        at java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:320) ~[?:?]
        at java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:722) ~[?:?]
        at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:510) ~[?:?]
        at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1773) ~[?:?]
        at java.util.concurrent.CompletableFuture$AsyncSupply.exec(CompletableFuture.java:1760) ~[?:?]
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:373) ~[?:?]
        at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1182) ~[?:?]
        at java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1655) ~[?:?]
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1622) ~[?:?]
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:165) ~[?:?]
Caused by: java.util.ConcurrentModificationException: java.util.ConcurrentModificationException
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77) ~[?:?]
        at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:?]
        at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499) ~[?:?]
        at java.lang.reflect.Constructor.newInstance(Constructor.java:480) ~[?:?]
        at java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:562) ~[?:?]
        at java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:591) ~[?:?]
        at java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:689) ~[?:?]
        at java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159) ~[?:?]
        at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(ForEachOps.java:173) ~[?:?]
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233) ~[?:?]
        at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596) ~[?:?]
        at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:765) ~[?:?]
        at dev.cheos.rolesync.Rolesync.lambda$syncPlayer$3(Rolesync.java:117) ~[?:?]
        at java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:718) ~[?:?]
        ... 8 more
Caused by: java.util.ConcurrentModificationException
        at java.util.ArrayList.sort(ArrayList.java:1723) ~[?:?]
[!]        at java.util.Collections.sort(Collections.java:145) ~[?:?]
[!]        at net.alpenblock.bungeeperms.PermissionsManager.addUserGroup(PermissionsManager.java:908) ~[?:?]
[!]        at net.alpenblock.bungeeperms.PermissionsManager.addUserGroup(PermissionsManager.java:888) ~[?:?]
[!]        at net.alpenblock.bungeeperms.BungeePermsAPI.userAddGroup(BungeePermsAPI.java:553) ~[?:?]
        at dev.cheos.rolesync.Rolesync.lambda$syncPlayer$2(Rolesync.java:119) ~[?:?]
        at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?]
        at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1850) ~[?:?]
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[?:?]
        at java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:290) ~[?:?]
        at java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:754) ~[?:?]
        ... 5 more

Additional context

  1. The issue could theoretically be circumvented by the calling class (of any 3rd party plugin using the bungeeperms api) by synchronizing onto bungeeperms internals which is (1) quite cumbersome and should not be required, and (2) might be quite difficult to get to work reliably.
  2. Due to the boxed nature of Minecraft plugin systems, this doesn't pose any harm to the running system, however (obviously) causes the api interaction to fail, and thus preventing some configuration/functionality of said 3rd party plugin(s) from working in these cases.
  3. dev.cheos.rolesync.Rolesync being a sample 3rd party plugin as mentioned

Cheos137 avatar Sep 10 '23 17:09 Cheos137

This is a valid issue; This requires a lot of internal changes in BungeePerms, and currently we don't have time to do those. We hope that we can lock at that in the near future, but we can't make any promises. If someone want's to get started on that, contributions are always welcome

aurorasmiles avatar Sep 11 '23 15:09 aurorasmiles