PlayPro / CoreProtect

CoreProtect is a blazing fast data logging and anti-griefing tool for Minecraft servers.
Artistic License 2.0
664 stars 339 forks source link

queueLookup API method throwing ConcurrentModificationException #481

Open Krakenied opened 10 months ago

Krakenied commented 10 months ago
List<String[]> queueLookup = api.queueLookup(block);

really often throws ConcurrentModificationException (probably especially on bigger servers)

[23:29:59 WARN]: java.util.ConcurrentModificationException
[23:29:59 WARN]:        at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
[23:29:59 WARN]:        at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
[23:29:59 WARN]:        at CoreProtect-22.2.jar//net.coreprotect.api.QueueLookup.performLookup(QueueLookup.java:52)
[23:29:59 WARN]:        at CoreProtect-22.2.jar//net.coreprotect.CoreProtectAPI.queueLookup(CoreProtectAPI.java:182)

The cause of the error is probably consumerData being modified while iterating. https://github.com/PlayPro/CoreProtect/blob/66b77ee75a24dac13f9cbf95242b504e1494dde7/src/main/java/net/coreprotect/api/QueueLookup.java#L52

Intelli commented 10 months ago

Switched the for loop for a ListIterator -- let me know if that resolves the issue.

Krakenied commented 8 months ago

@Intelli

It doesn't fix the issue. I wrote a simple class:

    public static void a() {
        final ArrayList<Object[]> list = new ArrayList<>();
        for (Object[] data : list) {
            System.out.println(data.length);
        }
    }

    public static void b() {
        final ArrayList<Object[]> list = new ArrayList<>();
        final ListIterator<Object[]> it = list.listIterator();
        while (it.hasNext()) {
            Object[] data = it.next();
            System.out.println(data.length);
        }
    }

And bytecode for both methods is nearly the same:

$ javap -c Test.class
Compiled from "Test.java"
public class Test {
  public Test();
    Code:
       0: aload_0
       1: invokespecial #1                  // Method java/lang/Object."<init>":()V
       4: return

  public static void a();
    Code:
       0: new           #7                  // class java/util/ArrayList
       3: dup
       4: invokespecial #9                  // Method java/util/ArrayList."<init>":()V
       7: astore_0
       8: aload_0
       9: invokevirtual #10                 // Method java/util/ArrayList.iterator:()Ljava/util/Iterator;
      12: astore_1
      13: aload_1
      14: invokeinterface #14,  1           // InterfaceMethod java/util/Iterator.hasNext:()Z
      19: ifeq          43
      22: aload_1
      23: invokeinterface #20,  1           // InterfaceMethod java/util/Iterator.next:()Ljava/lang/Object;
      28: checkcast     #24                 // class "[Ljava/lang/Object;"
      31: astore_2
      32: getstatic     #26                 // Field java/lang/System.out:Ljava/io/PrintStream;
      35: aload_2
      36: arraylength
      37: invokevirtual #32                 // Method java/io/PrintStream.println:(I)V
      40: goto          13
      43: return

  public static void b();
    Code:
       0: new           #7                  // class java/util/ArrayList
       3: dup
       4: invokespecial #9                  // Method java/util/ArrayList."<init>":()V
       7: astore_0
       8: aload_0
       9: invokevirtual #38                 // Method java/util/ArrayList.listIterator:()Ljava/util/ListIterator;
      12: astore_1
      13: aload_1
      14: invokeinterface #42,  1           // InterfaceMethod java/util/ListIterator.hasNext:()Z
      19: ifeq          43
      22: aload_1
      23: invokeinterface #45,  1           // InterfaceMethod java/util/ListIterator.next:()Ljava/lang/Object;
      28: checkcast     #24                 // class "[Ljava/lang/Object;"
      31: astore_2
      32: getstatic     #26                 // Field java/lang/System.out:Ljava/io/PrintStream;
      35: aload_2
      36: arraylength
      37: invokevirtual #32                 // Method java/io/PrintStream.println:(I)V
      40: goto          13
      43: return
}

Also, as you can see, ListIterator is fail-fast too: image It means, that will throw CME too: image I'm currently using a custom CoreProtect fork on my server to prevent this error from occurring. My solution to this is:

            for (Object[] data : consumerData.toArray(Object[][]::new)) {

and it hasn't thrown any errors since the change.

Krakenied commented 5 months ago

@Intelli still not fixed

Intelli commented 4 months ago

Can you provide the code you're using which triggers this error?

Krakenied commented 2 months ago

Still present https://github.com/LMBishop/Quests/issues/707

Can you provide the code you're using which triggers this error?

Crucial step: have dozens of players online

You just need to invoke CoreProtectAPI#queueLookup which calls QueueLookup#performLookup. And this thing throws: https://github.com/PlayPro/CoreProtect/blob/c493a0dece5780aa975ac5bf19899e3c369ebc9b/src/main/java/net/coreprotect/api/QueueLookup.java#L55

stale[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.