Ver Fonte

Fix: Improve exceptions produced by ThreadingDetector crashes

malte0811 há 3 anos atrás
pai
commit
ba7038e1c0

+ 27 - 15
common/src/main/java/malte0811/ferritecore/util/SmallThreadingDetector.java

@@ -6,6 +6,7 @@ import net.minecraft.util.ThreadingDetector;
 import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.BooleanSupplier;
 
 @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
 public class SmallThreadingDetector {
@@ -73,12 +74,12 @@ public class SmallThreadingDetector {
 
         private static void crashAcquire(SmallThreadDetectable owner) {
             var state = getAndWait(owner, ThreadRole.ACQUIRE);
-            throw ThreadingDetector.makeThreadingException(state.name, state.acquireThread);
+            throw state.mainException;
         }
 
         private static void crashRelease(SmallThreadDetectable owner) {
             var state = getAndWait(owner, ThreadRole.RELEASE);
-            throw ThreadingDetector.makeThreadingException(state.name, state.releaseThread);
+            throw state.mainException;
         }
 
         private static void crashBystander(SmallThreadDetectable owner) {
@@ -110,6 +111,7 @@ public class SmallThreadingDetector {
         final String name;
         Thread acquireThread;
         Thread releaseThread;
+        RuntimeException mainException;
 
         private CrashingState(String name) {
             this.name = name;
@@ -126,24 +128,34 @@ public class SmallThreadingDetector {
             // Notify other threads waiting for this crash to be ready
             notifyAll();
             try {
-                final long maxTotalTime = 10000;
-                final var start = System.currentTimeMillis();
-                while (acquireThread == null || releaseThread == null) {
-                    if (System.currentTimeMillis() - start > maxTotalTime) {
-                        // Crash without both threads present if we don't manage to "find" them within 10 seconds
-                        // Happens e.g. when a release call is just missing, vanilla would hang indefinitely instead
-                        // in this case
-                        throw new RuntimeException(
-                                "Threading detector crash did not find other thread, missing release call?"
-                        );
-                    }
-                    // Release lock on this for up to 10 seconds, or until the other threads are ready
-                    this.wait(maxTotalTime);
+                waitUntilOrCrash(() -> acquireThread != null && releaseThread != null);
+                if (role == ThreadRole.ACQUIRE) {
+                    mainException = ThreadingDetector.makeThreadingException(name, releaseThread);
+                    notifyAll();
+                } else {
+                    waitUntilOrCrash(() -> mainException != null);
                 }
             } catch (InterruptedException x) {
                 Thread.currentThread().interrupt();
             }
         }
+
+        private synchronized void waitUntilOrCrash(BooleanSupplier isReady) throws InterruptedException {
+            final long maxTotalTime = 10000;
+            final var start = System.currentTimeMillis();
+            while (!isReady.getAsBoolean()) {
+                if (System.currentTimeMillis() - start > maxTotalTime) {
+                    // Crash without both threads present if we don't manage to "find" them within 10 seconds
+                    // Happens e.g. when a release call is just missing, vanilla would hang indefinitely instead
+                    // in this case
+                    throw new RuntimeException(
+                            "Threading detector crash did not find other thread, missing release call?"
+                    );
+                }
+                // Release lock on this for up to 10 seconds, or until the other threads are ready
+                this.wait(maxTotalTime);
+            }
+        }
     }
 
     private enum ThreadRole {

+ 7 - 2
common/src/test/java/malte0811/ferritecore/util/SmallThreadingDetectorTest.java

@@ -8,6 +8,8 @@ import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.function.Executable;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.CyclicBarrier;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
@@ -49,15 +51,18 @@ class SmallThreadingDetectorTest {
     public void testRace() throws InterruptedException {
         var obj = new OwnedObject();
         AtomicBoolean anyTripped = new AtomicBoolean(false);
+        List<Thread> threads = new ArrayList<>(10);
         for (int i = 0; i < 10; ++i)
-            runOnNewThread(() -> {
+            threads.add(runOnNewThread(() -> {
                 final long start = System.currentTimeMillis();
                 while (!anyTripped.get() && System.currentTimeMillis() - start < 1000) {
                     SmallThreadingDetector.acquire(obj, "test");
                     SmallThreadingDetector.release(obj);
                 }
-            }, $ -> anyTripped.set(true));
+            }, $ -> anyTripped.set(true)));
         Thread.sleep(1000);
+        for (var thread : threads)
+            thread.join();
         Assertions.assertTrue(anyTripped.get());
     }