From 5ab8d50b86547457c82a470f7a21735a3a5f08fd Mon Sep 17 00:00:00 2001
From: SirYwell <hannesgreule@outlook.de>
Date: Fri, 6 Aug 2021 10:33:38 +0200
Subject: [PATCH] Don't keep PlotFlagUpdateHandlers forever This allows Plots,
 FlagContainers and its PlotFlagUpdateHandlers being cleaned up by the GC
 correctly

---
 .../java/com/plotsquared/core/plot/Plot.java  |  5 +++
 .../core/plot/flag/FlagContainer.java         | 32 ++++++++++++++++---
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/Core/src/main/java/com/plotsquared/core/plot/Plot.java b/Core/src/main/java/com/plotsquared/core/plot/Plot.java
index e039b6cad..37e8e4f3c 100644
--- a/Core/src/main/java/com/plotsquared/core/plot/Plot.java
+++ b/Core/src/main/java/com/plotsquared/core/plot/Plot.java
@@ -85,6 +85,7 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import java.lang.ref.Cleaner;
 import java.text.DecimalFormat;
 import java.text.SimpleDateFormat;
 import java.util.ArrayDeque;
@@ -127,6 +128,7 @@ public class Plot {
     private static final Logger LOGGER = LogManager.getLogger("PlotSquared/" + Plot.class.getSimpleName());
     private static final DecimalFormat FLAG_DECIMAL_FORMAT = new DecimalFormat("0");
     private static final MiniMessage MINI_MESSAGE = MiniMessage.builder().build();
+    private static final Cleaner CLEANER = Cleaner.create();
 
     static Set<Plot> connected_cache;
     static Set<CuboidRegion> regions_cache;
@@ -256,6 +258,9 @@ public class Plot {
         this.temp = temp;
         this.flagContainer.setParentContainer(area.getFlagContainer());
         PlotSquared.platform().injector().injectMembers(this);
+        // This is needed, because otherwise the Plot, the FlagContainer and its
+        // `this::handleUnknown` PlotFlagUpdateHandler won't get cleaned up ever
+        CLEANER.register(this, this.flagContainer.createCleanupHook());
     }
 
     /**
diff --git a/Core/src/main/java/com/plotsquared/core/plot/flag/FlagContainer.java b/Core/src/main/java/com/plotsquared/core/plot/flag/FlagContainer.java
index ceca113ce..e4a0e8b0f 100644
--- a/Core/src/main/java/com/plotsquared/core/plot/flag/FlagContainer.java
+++ b/Core/src/main/java/com/plotsquared/core/plot/flag/FlagContainer.java
@@ -27,14 +27,15 @@ package com.plotsquared.core.plot.flag;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
-import org.checkerframework.checker.nullness.qual.NonNull;
-import org.checkerframework.checker.nullness.qual.Nullable;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.jetbrains.annotations.ApiStatus;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
@@ -49,8 +50,9 @@ public class FlagContainer {
     private final Map<String, String> unknownFlags = new HashMap<>();
     private final Map<Class<?>, PlotFlag<?, ?>> flagMap = new HashMap<>();
     private final PlotFlagUpdateHandler plotFlagUpdateHandler;
-    private final Collection<PlotFlagUpdateHandler> updateSubscribers = new ArrayList<>();
+    private final Collection<PlotFlagUpdateHandler> updateSubscribers = new HashSet<>();
     private FlagContainer parentContainer;
+    private final PlotFlagUpdateHandler unknownsRef;
 
     /**
      * Construct a new flag container with an optional parent container and update handler.
@@ -71,7 +73,10 @@ public class FlagContainer {
         this.parentContainer = parentContainer;
         this.plotFlagUpdateHandler = plotFlagUpdateHandler;
         if (!(this instanceof GlobalFlagContainer)) {
-            GlobalFlagContainer.getInstance().subscribe(this::handleUnknowns);
+            this.unknownsRef = this::handleUnknowns;
+            GlobalFlagContainer.getInstance().subscribe(this.unknownsRef);
+        } else {
+            this.unknownsRef = null;
         }
     }
 
@@ -336,6 +341,23 @@ public class FlagContainer {
         this.unknownFlags.put(flagName.toLowerCase(Locale.ENGLISH), value);
     }
 
+    /**
+     * Creates a cleanup hook that is meant to run once this FlagContainer isn't needed anymore.
+     * This is to prevent memory leaks. This method is not part of the API.
+     *
+     * @return a new Runnable that cleans up once the FlagContainer isn't needed anymore.
+     */
+    @ApiStatus.Internal
+    public Runnable createCleanupHook() {
+        return () -> GlobalFlagContainer.getInstance().unsubscribe(unknownsRef);
+    }
+
+    void unsubscribe(final @Nullable PlotFlagUpdateHandler updateHandler) {
+        if (updateHandler != null) {
+            this.updateSubscribers.remove(updateHandler);
+        }
+    }
+
     public boolean equals(final Object o) {
         if (o == this) {
             return true;