Fix possibly huge memory leak caused by long lived CancellableManager…

…Provider (#213)

* Fix forever growing cancellable Manager

A long lived CancellableManagerProvider could easily end up with very many “cancelled” Cancellable in it’s internal CancellableManager because this is only cleared when the Provider itself is cancelled.

This can cause huge memory leaks in some cases, for example in the DebounceProcessor

Since there is only ever one “not cancelled” CancellableManager provided by the CancellableManagerProvider, there is actually no need to keep the previously provided, now cancelled, CancellableManager.

We now simply use the internalCancellableManagerRef.

Also took the oportunity to make cancelPreviousAndCreate hopefully atomic be exposing and using getAndSet on AtomicReference

* Add test for “already cancelled” contract and implement

* remove another hole

* Update public API declaration

trikot-foundation/trikotFoundation/api/android/trikotFoundation.api

@@ -203,6 +203,7 @@ public final class com/mirego/trikot/foundation/concurrent/AtomicReference {
 	public fun <init> (Ljava/lang/Object;)V
 	public final fun compareAndSet (Ljava/lang/Object;Ljava/lang/Object;)Z
 	public final fun compareAndSwap (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
+	public final fun getAndSet (Ljava/lang/Object;)Ljava/lang/Object;
 	public final fun getValue ()Ljava/lang/Object;
 	public final fun setOrThrow (Ljava/lang/Object;Ljava/lang/Object;)V
 	public final fun setOrThrow (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function0;)V

trikot-foundation/trikotFoundation/api/jvm/trikotFoundation.api

@@ -203,6 +203,7 @@ public final class com/mirego/trikot/foundation/concurrent/AtomicReference {
 	public fun <init> (Ljava/lang/Object;)V
 	public final fun compareAndSet (Ljava/lang/Object;Ljava/lang/Object;)Z
 	public final fun compareAndSwap (Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;
+	public final fun getAndSet (Ljava/lang/Object;)Ljava/lang/Object;
 	public final fun getValue ()Ljava/lang/Object;
 	public final fun setOrThrow (Ljava/lang/Object;Ljava/lang/Object;)V
 	public final fun setOrThrow (Ljava/lang/Object;Ljava/lang/Object;Lkotlin/jvm/functions/Function0;)V

trikot-foundation/trikotFoundation/src/commonMain/kotlin/com/mirego/trikot/foundation/concurrent/AtomicReference.kt

@@ -23,6 +23,10 @@ class AtomicReference<T>(value: T) {
     fun compareAndSwap(expected: T, new: T): T {
         return if (compareAndSet(expected, new)) new else value
     }
+
+    fun getAndSet(new: T): T {
+        return atomicValue.getAndSet(new)
+    }
 }
 
 fun <T> AtomicReference<T>.setOrThrow(new: T) = setOrThrow(value, new)

trikot-streams/streams/src/commonMain/kotlin/com/mirego/trikot/streams/cancellable/CancellableManagerProvider.kt

@@ -1,20 +1,29 @@
 package com.mirego.trikot.streams.cancellable
 
 import com.mirego.trikot.foundation.concurrent.AtomicReference
+import com.mirego.trikot.foundation.concurrent.dispatchQueue.SynchronousSerialQueue
 
 class CancellableManagerProvider : Cancellable {
-    private val cancellableManager = CancellableManager()
+    private val serialQueue = SynchronousSerialQueue()
     private val internalCancellableManagerRef = AtomicReference(CancellableManager())
+    private val isCancelled = AtomicReference(false)
 
     fun cancelPreviousAndCreate(): CancellableManager {
-        internalCancellableManagerRef.value.cancel()
-        return CancellableManager().also {
-            internalCancellableManagerRef.setOrThrow(internalCancellableManagerRef.value, it)
-            cancellableManager.add(it)
+        return CancellableManager().also { cancellableManager ->
+            internalCancellableManagerRef.getAndSet(cancellableManager).cancel()
+            serialQueue.dispatch {
+                if (isCancelled.value) {
+                    cancellableManager.cancel()
+                }
+            }
         }
     }
 
     override fun cancel() {
-        cancellableManager.cancel()
+        serialQueue.dispatch {
+            if (isCancelled.compareAndSet(isCancelled.value, true)) {
+                internalCancellableManagerRef.value.cancel()
+            }
+        }
     }
 }

trikot-streams/streams/src/commonTest/kotlin/com/mirego/trikot/streams/cancellable/CancellableManagerProviderTests.kt

@@ -24,4 +24,15 @@ class CancellableManagerProviderTests {
 
         assertTrue { cancelled }
     }
+
+    @Test
+    fun requestingACancellableManagerFromACancelledProviderReturnsACancelledCancellableManager() {
+        val cancellableManagerProvider = CancellableManagerProvider()
+        cancellableManagerProvider.cancel()
+        val cancellableManager = cancellableManagerProvider.cancelPreviousAndCreate()
+        var cancelled = false
+        cancellableManager.add { cancelled = true }
+
+        assertTrue { cancelled }
+    }
 }