From 06fe9ccda0fc0620a9152e8603cf55652c64c51f Mon Sep 17 00:00:00 2001
From: malte0811 <malte0811@web.de>
Date: Thu, 29 Oct 2020 18:09:20 +0100
Subject: [PATCH] Fix (NonNull)Lazy.Concurrent using a global lock (#7403)

* Fix Lazy.Concurrent using a global lock, thus preventing multiple threads from resolving independent Lazy's at the same time
* Use Lazy with an added null check to implement NonNullLazy
---
 .../net/minecraftforge/common/util/Lazy.java  | 10 ++-
 .../common/util/NonNullLazy.java              | 68 ++-----------------
 2 files changed, 13 insertions(+), 65 deletions(-)

diff --git a/src/main/java/net/minecraftforge/common/util/Lazy.java b/src/main/java/net/minecraftforge/common/util/Lazy.java
index fe1c4e96e..58368d65a 100644
--- a/src/main/java/net/minecraftforge/common/util/Lazy.java
+++ b/src/main/java/net/minecraftforge/common/util/Lazy.java
@@ -78,7 +78,7 @@ public interface Lazy<T> extends Supplier<T>
      */
     final class Concurrent<T> implements Lazy<T>
     {
-        private static final Object lock = new Object();
+        private volatile Object lock = new Object();
         private volatile Supplier<T> supplier;
         private volatile T instance;
 
@@ -91,14 +91,20 @@ public interface Lazy<T> extends Supplier<T>
         @Override
         public final T get()
         {
+            // Copy the lock to a local variable to prevent NPEs if the lock field is set to null between the
+            // null-check and the synchronization
+            Object localLock = this.lock;
             if (supplier != null)
             {
-                synchronized (lock)
+                // localLock is not null here because supplier was non-null after we copied the lock and both of them
+                // are volatile
+                synchronized (localLock)
                 {
                     if (supplier != null)
                     {
                         instance = supplier.get();
                         supplier = null;
+                        this.lock = null;
                     }
                 }
             }
diff --git a/src/main/java/net/minecraftforge/common/util/NonNullLazy.java b/src/main/java/net/minecraftforge/common/util/NonNullLazy.java
index 23a652703..cbed18822 100644
--- a/src/main/java/net/minecraftforge/common/util/NonNullLazy.java
+++ b/src/main/java/net/minecraftforge/common/util/NonNullLazy.java
@@ -20,8 +20,7 @@
 package net.minecraftforge.common.util;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-import java.util.function.Supplier;
+import java.util.Objects;
 
 /**
  * Proxy object for a value that is calculated on first access.
@@ -36,7 +35,8 @@ public interface NonNullLazy<T> extends NonNullSupplier<T>
      */
     static <T> NonNullLazy<T> of(@Nonnull NonNullSupplier<T> supplier)
     {
-        return new NonNullLazy.Fast<>(supplier);
+        Lazy<T> lazy = Lazy.of(supplier::get);
+        return () -> Objects.requireNonNull(lazy.get());
     }
 
     /**
@@ -45,65 +45,7 @@ public interface NonNullLazy<T> extends NonNullSupplier<T>
      */
     static <T> NonNullLazy<T> concurrentOf(@Nonnull NonNullSupplier<T> supplier)
     {
-        return new NonNullLazy.Concurrent<>(supplier);
-    }
-
-    /**
-     * Non-thread-safe implementation.
-     */
-    final class Fast<T> implements NonNullLazy<T>
-    {
-        private NonNullSupplier<T> supplier;
-        private T instance;
-
-        private Fast(NonNullSupplier<T> supplier)
-        {
-            this.supplier = supplier;
-        }
-
-        @Nonnull
-        @Override
-        public final T get()
-        {
-            if (supplier != null)
-            {
-                instance = supplier.get();
-                supplier = null;
-            }
-            return instance;
-        }
-    }
-
-    /**
-     * Thread-safe implementation.
-     */
-    final class Concurrent<T> implements NonNullLazy<T>
-    {
-        private static final Object lock = new Object();
-        private volatile NonNullSupplier<T> supplier;
-        private volatile T instance;
-
-        private Concurrent(NonNullSupplier<T> supplier)
-        {
-            this.supplier = supplier;
-        }
-
-        @Nonnull
-        @Override
-        public final T get()
-        {
-            if (supplier != null)
-            {
-                synchronized (lock)
-                {
-                    if (supplier != null)
-                    {
-                        instance = supplier.get();
-                        supplier = null;
-                    }
-                }
-            }
-            return instance;
-        }
+        Lazy<T> lazy = Lazy.concurrentOf(supplier::get);
+        return () -> Objects.requireNonNull(lazy.get());
     }
 }