Fix event subscription memory leak

Distribution/wwDotnetBridge.PRG

@@ -1838,34 +1838,26 @@ this.oBridge.InvokeMethodAsync(this,this.oSubscriber,"WaitForEvent")
 ENDFUNC
 
 ************************************************************************
-*  OnComplete
+*  OnCompleted
 ****************************************
 ***  Function: Event Proxy that forwards the event to a function 
 ***            named On{Event} with event's parameters.
 ************************************************************************
 FUNCTION OnCompleted(lvResult, lcMethod)
-LOCAL loParams,lParamText,lCount
+LOCAL lParamText, lCount
 
-IF ISNULL(lvResult) && If the call to WaitForEvent was canceled:
+IF VARTYPE(lvResult) != "O" OR VARTYPE(THIS.oHandler) != "O" && If the call to WaitForEvent was canceled or the handler was unsubscribed:
+	* Note that lvResult is sometimes an empty string. This is likely the result of a race condition. Fortunately, checking whether it is an object seems to be a reliable workaround.
 	RETURN
 ENDIF
 
-
-loParams=CREATEOBJECT("EMPTY") && Workaround to index into array of parameters. 
 lParamText = ""
-IF NOT ISNULL(lvResult.Params)
-	lCount = 0
-	FOR EACH lParam IN lvResult.Params
-		lCount = lCount + 1
-		AddProperty(loParams,"P" + ALLTRIM(STR(lCount)),lParam)
-		lParamText = lParamText + ",loParams.P" + ALLTRIM(STR(lCount))
-	ENDFOR
-ENDIF
+FOR lCount = 0 TO lvResult.Params.Count - 1
+	lParamText = lParamText + ",lvResult.Params.Item(" + ALLTRIM(STR(lCount)) + ")"
+ENDFOR
 
-IF VARTYPE(THIS.oHandler) = "O"
-	=EVALUATE("this.oHandler." + this.oPrefix + lvResult.Name + "("+SUBSTR(lParamText,2)+")")
-	this.HandleNextEvent()
-ENDIF
+=EVALUATE("this.oHandler." + this.oPrefix + lvResult.Name + "("+SUBSTR(lParamText,2)+")")
+this.HandleNextEvent()
 
 ENDFUNC
 

DotnetBridge/Utilities/EventSubscriber.cs

@@ -1,10 +1,12 @@
 using System;
+using System.Collections;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 
 namespace Westwind.WebConnection
@@ -18,13 +20,10 @@ public sealed class EventSubscriber : IDisposable
         private readonly object _source;
         private readonly List<DelegateInfo> _eventHandlers = new List<DelegateInfo>();
         private readonly ConcurrentQueue<RaisedEvent> _raisedEvents = new ConcurrentQueue<RaisedEvent>();
-        private TaskCompletionSource<RaisedEvent> _completion = new TaskCompletionSource<RaisedEvent>();
+        private volatile SemaphoreSlim _signal = new SemaphoreSlim(0);
 
         public EventSubscriber(object source, String prefix = "", dynamic vfp = null)
         {
-            // Indicates that initially the client is not waiting.
-            _completion.SetResult(null);
-
             // For each event, adds a handler that calls QueueInteropEvent.
             _source = source;
             foreach (var ev in source.GetType().GetEvents())
@@ -62,16 +61,37 @@ public DelegateInfo(Delegate handler, EventInfo eventInfo)
 
         public void Dispose()
         {
+            if (_signal == null)
+                return;
+
+            // Unsubscribe from all events.
             foreach (var item in _eventHandlers)
                 item.EventInfo.RemoveEventHandler(_source, item.Delegate);
-            _completion.TrySetCanceled();
+
+            // Remove references to objects.
+            _eventHandlers.Clear();
+            while (_raisedEvents.TryDequeue(out _)) { }
+
+            // Release any waiting thread and dispose the semaphore.
+            _signal.Release();
+            _signal.Dispose();
+            _signal = null;
         }
 
         private void QueueInteropEvent(string name, object[] parameters)
         {
-            var interopEvent = new RaisedEvent { Name = name, Params = parameters };
-            if (!_completion.TrySetResult(interopEvent))
-                _raisedEvents.Enqueue(interopEvent);
+            var signal = _signal;
+            if (signal == null)
+                return;
+
+            // Push event to queue
+            var parametersArrayList = new ArrayList(parameters.Length);
+            parametersArrayList.AddRange(parameters);
+            var interopEvent = new RaisedEvent { Name = name, Params = parametersArrayList };
+            _raisedEvents.Enqueue(interopEvent);
+
+            // Release waiting thread
+            signal.Release();
         }
 
         /// <summary>
@@ -80,19 +100,21 @@ private void QueueInteropEvent(string name, object[] parameters)
         /// <returns>The next event, or null if this subscriber has been disposed.</returns>
         public RaisedEvent WaitForEvent()
         {
-            if (_raisedEvents.TryDequeue(out var interopEvent)) return interopEvent;
-            _completion = new TaskCompletionSource<RaisedEvent>();
-            var task = _completion.Task;
-
-            task.Wait();
-
-            return task.IsCanceled ? null : task.Result;
+            var signal = _signal;
+            if (signal == null)
+                return null;
+
+            // Wait and return the event.
+            // Since signal keeps a count, there will always be exactly one event to dequeue, except when the subscriber is disposed, in which case there will be none.
+            try { signal.Wait(); } catch (ObjectDisposedException) { }
+            _raisedEvents.TryDequeue(out var interopEvent);
+            return interopEvent;
         }
     }
 
     public class RaisedEvent
     {
         public string Name { get; internal set; }
-        public object[] Params { get; internal set; }
+        public ArrayList Params { get; internal set; }
     }
 }

Tests/EventSubscriberTests.cs

@@ -35,9 +35,9 @@ public void EventSubscriber_WaitForFutureEvents()
         static void VerifyResults(EventSubscriber subscriber)
         {
             var result = subscriber.WaitForEvent();
-            Assert.IsTrue(result.Name == nameof(Loopback.NoParams) && result.Params.Length == 0);
+            Assert.IsTrue(result.Name == nameof(Loopback.NoParams) && result.Params.Count == 0);
             result = subscriber.WaitForEvent();
-            Assert.IsTrue(result.Name == nameof(Loopback.TwoParams) && result.Params.Length == 2 && (string)result.Params[0] == "A" && (int)result.Params[1] == 1);
+            Assert.IsTrue(result.Name == nameof(Loopback.TwoParams) && result.Params.Count == 2 && (string)result.Params[0] == "A" && (int)result.Params[1] == 1);
         }
     }