Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: BungeePermsAPI not thread safe #402

Open
Cheos137 opened this issue Sep 10, 2023 · 1 comment
Open

[Bug]: BungeePermsAPI not thread safe #402

Cheos137 opened this issue Sep 10, 2023 · 1 comment

Comments

@Cheos137
Copy link
Contributor

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
@aurorasmiles
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants