From 00c3a15a8d4821c4638506b104977c046d4a57ea Mon Sep 17 00:00:00 2001
From: Arnaud Bouchez <ab@synopse.info>
Date: Mon, 25 Jul 2022 17:33:04 +0200
Subject: [PATCH] removed internal list of unsubscribed tags for async sockets
 - this list intend was an attempt to stabilize HTTP/1.0 process, but it was
 in fact not a good idea at all :( - it triggered random GPF and memory leaks
 on highly-concurrent HTTP/1.0 connections, because TAsyncConnection pointers
 were reused by the MM whereas the pending results tags were still not
 flushed, so those new connections were in fact ignored... and eventually
 leaked... - to be fair, it was not a very realistic use case, but "wrk -c
 16384" was able to reproduce it, if you set the server keep alive parameter
 to 1 second, or with Apache Bench in HTTP/1.0 mode - resulting code seems now
 pretty stable from HTTP/1.1 or HTTP/1.0 requests - as a side benefit, it also
 enhances performance, because we don't have to make an O(n) over this list
 any more

---
 src/mormot.commit.inc        |  2 +-
 src/net/mormot.net.async.pas |  2 +-
 src/net/mormot.net.sock.pas  | 27 ++++++++-------------------
 3 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/src/mormot.commit.inc b/src/mormot.commit.inc
index 4878cbb2d..f4402cb0a 100644
--- a/src/mormot.commit.inc
+++ b/src/mormot.commit.inc
@@ -1 +1 @@
-'2.0.3762'
+'2.0.3763'
diff --git a/src/net/mormot.net.async.pas b/src/net/mormot.net.async.pas
index 17ffbcdab..cd2dc1f5f 100644
--- a/src/net/mormot.net.async.pas
+++ b/src/net/mormot.net.async.pas
@@ -1076,7 +1076,7 @@ function TPollAsyncSockets.Start(connection: TPollAsyncConnection): boolean;
 end;
 
 const
-  _SUB: array[boolean] of AnsiChar = '+-';
+  _SUB: array[boolean] of AnsiChar = '-+';
 
 function TPollAsyncSockets.Stop(connection: TPollAsyncConnection): boolean;
 var
diff --git a/src/net/mormot.net.sock.pas b/src/net/mormot.net.sock.pas
index fa96359e5..0d9368acf 100644
--- a/src/net/mormot.net.sock.pas
+++ b/src/net/mormot.net.sock.pas
@@ -558,7 +558,7 @@ procedure InitNetTlsContext(var TLS: TNetTlsContext; Server: boolean = false;
   /// set of events monitored by TPollSocketAbstract
   TPollSocketEvents = set of TPollSocketEvent;
 
-  /// some opaque value (which may be a pointer) associated with a polling event
+  /// some opaque value (typically a pointer) associated with a polling event
   TPollSocketTag = type PtrInt;
   PPollSocketTag = ^TPollSocketTag;
   TPollSocketTagDynArray = TPtrUIntDynArray;
@@ -685,14 +685,12 @@   TPollSockets = class(TPollAbstract)
     fPendingSafe: TLightLock;
     fPollIndex: integer;
     fGettingOne: integer;
-    fLastUnsubscribeTagCount: integer;
     fTerminated: boolean;
     fEpollGettingOne: boolean;
     fUnsubscribeShouldShutdownSocket: boolean;
     fPollClass: TPollSocketClass;
     fOnLog: TSynLogProc;
     fOnGetOneIdle: TOnPollSocketsIdle;
-    fLastUnsubscribeTag: TPollSocketTagDynArray; // protected by fPendingSafe
     // used for select/poll (FollowEpoll=false) with multiple thread-unsafe fPoll[]
     fSubscription: TPollSocketsSubscription;
     fSubscriptionSafe: TLightLock; // dedicated not to block Accept()
@@ -2457,7 +2455,6 @@ procedure TPollSockets.Unsubscribe(socket: TNetSocket; tag: TPollSocketTag);
         if fnd <> nil then
           byte(fnd^.events) := 0; // GetOnePending() will ignore it
       end;
-      AddPtrUInt(fLastUnsubscribeTag, fLastUnsubscribeTagCount, tag);
     finally
       fPendingSafe.UnLock;
     end;
@@ -2487,7 +2484,7 @@ procedure TPollSockets.Unsubscribe(socket: TNetSocket; tag: TPollSocketTag);
 
 function TPollSockets.IsValidPending(tag: TPollSocketTag): boolean;
 begin
-  result := true; // overriden e.g. in TPollAsyncReadSockets
+  result := tag <> 0; // overriden e.g. in TPollAsyncReadSockets
 end;
 
 function TPollSockets.GetSubscribeCount: integer;
@@ -2512,8 +2509,6 @@ function TPollSockets.GetOnePending(out notif: TPollSocketResult;
   const call: RawUtf8): boolean;
 var
   n, ndx: PtrInt;
-label
-  ok;
 begin
   result := false;
   if fTerminated or
@@ -2528,31 +2523,25 @@ function TPollSockets.GetOnePending(out notif: TPollSocketResult;
     n := fPending.Count;
     ndx := fPendingIndex;
     if ndx < n then
-    begin
       repeat
         // retrieve next notified event
         notif := fPending.Events[ndx];
         // move forward
         inc(ndx);
-        if (byte(notif.events) <> 0) and // Unsubscribe() may have reset to 0
-           IsValidPending(notif.tag) and // e.g. TPollAsyncReadSockets
-           ((fLastUnsubscribeTagCount = 0) or
-            not PtrUIntScanExists(pointer(fLastUnsubscribeTag),
-              fLastUnsubscribeTagCount, notif.tag)) then
+        if (byte(notif.events) <> 0) and  // Unsubscribe() may have reset to 0
+           IsValidPending(notif.tag) then // e.g. TPollAsyncReadSockets
         begin
           // there is a non-void event to return
           result := true;
           fPendingIndex := ndx; // continue with next event
-          // quick exit with one notified event
-          if ndx = n then
-            break; // reset fPending list
-          goto ok;
+          break;
         end;
       until ndx >= n;
+    if ndx >= n then
+    begin
       fPending.Count := 0; // reuse shared fPending.Events[] memory
       fPendingIndex := 0;
-      fLastUnsubscribeTagCount := 0;
-ok: end;
+    end;
   {$ifdef HASFASTTRYFINALLY}
   finally
   {$endif HASFASTTRYFINALLY}