From 25e25b0f86a30666950d465ecb9303d0bb88f7da Mon Sep 17 00:00:00 2001
From: DBotThePony <dbotthepony@yandex.ru>
Date: Tue, 25 Feb 2025 16:09:32 +0700
Subject: [PATCH] Remove structural synchronization inside SynchableGroup Also
 fixed memory leak when removing remotes

---
 .../network/syncher/DynamicSynchableGroup.kt  |  4 +-
 .../mc/otm/network/syncher/SynchableGroup.kt  | 74 +++++++++----------
 2 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/DynamicSynchableGroup.kt b/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/DynamicSynchableGroup.kt
index fb3d3ba6e..ebfcf27dd 100644
--- a/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/DynamicSynchableGroup.kt
+++ b/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/DynamicSynchableGroup.kt
@@ -180,7 +180,7 @@ class DynamicSynchableGroup<T : ISynchable>(
 
 	private data class Slot<T : Any>(val synchable: T, val id: Int)
 
-	private val remoteStates = CopyOnWriteArrayList<RemoteState>()
+	private val remoteStates = ArrayList<RemoteState>()
 	private val value2slot = HashMap<T, Slot<T>>()
 	private val id2slot = Int2ObjectOpenHashMap<Slot<T>>()
 	private val idAllocator = IDAllocator()
@@ -271,7 +271,7 @@ class DynamicSynchableGroup<T : ISynchable>(
 			value2slot.clear()
 			id2slot.clear()
 
-			remoteStates.forEach { it.clear() }
+			remoteStates.toTypedArray().forEach { it.clear() }
 		}
 	}
 
diff --git a/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/SynchableGroup.kt b/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/SynchableGroup.kt
index e61c14779..88813c500 100644
--- a/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/SynchableGroup.kt
+++ b/src/main/kotlin/ru/dbotthepony/mc/otm/network/syncher/SynchableGroup.kt
@@ -40,6 +40,7 @@ import java.util.function.DoubleSupplier
 import java.util.function.IntSupplier
 import java.util.function.LongSupplier
 import java.util.function.Supplier
+import kotlin.collections.ArrayList
 import kotlin.concurrent.withLock
 
 /**
@@ -56,7 +57,6 @@ import kotlin.concurrent.withLock
  */
 @Suppress("UNUSED")
 class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
-	private val lock = ReentrantLock()
 	private val slots = ArrayList<Slot?>()
 	private val gaps = IntAVLTreeSet()
 	private val observers = ArrayList<Slot>()
@@ -109,7 +109,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 		val id: Int
 
 		private var isRemoved = false
-		val remoteSlots = CopyOnWriteArrayList<Remote.RemoteSlot>()
+		val remoteSlots = ArrayList<Remote.RemoteSlot>()
 
 		override fun close() {
 			if (!isRemoved) {
@@ -118,7 +118,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 				gaps.add(id)
 
 				observers.remove(this)
-				remoteSlots.forEach { it.remove() }
+				remoteSlots.toTypedArray().forEach { it.remove() }
 			}
 		}
 
@@ -128,32 +128,28 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 
 		private fun markDirty() {
 			if (!isRemote && !isRemoved) {
-				remoteSlots.forEach {
-					it.markDirty()
-				}
+				remoteSlots.forEach { it.markDirty() }
 			}
 		}
 
 		init {
-			lock.withLock {
-				if (synchable.shouldBeObserved) {
-					observers.add(this)
-				}
+			if (synchable.shouldBeObserved) {
+				observers.add(this)
+			}
 
-				if (gaps.isNotEmpty()) {
-					val gap = gaps.firstInt()
-					gaps.remove(gap)
-					id = gap
-					check(slots[id] == null) { "Expected slot $id to be empty" }
-					slots[id] = this
-				} else {
-					id = slots.size
-					slots.add(this)
-				}
+			if (gaps.isNotEmpty()) {
+				val gap = gaps.firstInt()
+				gaps.remove(gap)
+				id = gap
+				check(slots[id] == null) { "Expected slot $id to be empty" }
+				slots[id] = this
+			} else {
+				id = slots.size
+				slots.add(this)
+			}
 
-				remotes.forEach {
-					it.addSlot(this)
-				}
+			remotes.forEach {
+				it.addSlot(this)
 			}
 		}
 	}
@@ -301,12 +297,11 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 	 * To get changes to be networked to remote client, [write] should be called.
 	 */
 	inner class Remote(private val listener: Runnable) : Closeable, IRemoteState {
-		constructor() : this(Runnable {  })
+		constructor() : this(Runnable {})
 
-		@Volatile
 		private var isRemoved = false
 		internal val dirty = ConcurrentLinkedQueue<RemoteSlot>()
-		private val remoteSlots = CopyOnWriteArrayList<RemoteSlot>()
+		private val remoteSlots = ArrayList<RemoteSlot>()
 
 		internal inner class RemoteSlot(val slot: Slot) {
 			private val isDirty = AtomicBoolean()
@@ -357,6 +352,7 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 				remoteSlots.remove(this)
 				dirty.remove(this)
 				cleanable.clean()
+				slot.remoteSlots.remove(this)
 			}
 
 			override fun hashCode(): Int {
@@ -369,15 +365,13 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 		}
 
 		init {
-			lock.withLock {
-				slots.forEach {
-					if (it != null) {
-						addSlot(it)
-					}
+			slots.forEach {
+				if (it != null) {
+					addSlot(it)
 				}
-
-				remotes.add(this)
 			}
+
+			remotes.add(this)
 		}
 
 		internal fun addSlot(slot: Slot) {
@@ -435,15 +429,13 @@ class SynchableGroup : Observer, ISynchable, Iterable<ISynchable> {
 			if (!isRemoved) {
 				isRemoved = true
 
-				lock.withLock {
-					remoteSlots.forEach {
-						it.remove()
-						it.slot.remoteSlots.remove(it)
-					}
-
-					remotes.remove(this)
-					dirty.clear()
+				remoteSlots.toTypedArray().forEach {
+					it.remove()
+					it.slot.remoteSlots.remove(it)
 				}
+
+				remotes.remove(this)
+				dirty.clear()
 			}
 		}
 	}