fixes around reverse client with timings

radkesvat · May 28, 2024 · 04aa5ec · 04aa5ec
1 parent 3c3270c
commit 04aa5ec
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 11 deletions.
diff --git a/tunnels/client/reverse/helpers.h b/tunnels/client/reverse/helpers.h
@@ -14,8 +14,8 @@ enum
 {
     kPreconnectDelayShort                              = 10,
     kPreconnectDelayHigh                               = 750,
-    kConnectionStarvationTimeOut                       = 5 * 1000,
-    kConnectionStarvationTimeOutAfterFirstConfirmation = 25 * 1000
+    kConnectionStarvationTimeOut                       = 3000,
+    kConnectionStarvationTimeOutAfterFirstConfirmation = 15000
 };
 
 static void onLinePausedU(void *cstate)
@@ -142,17 +142,29 @@ static void initiateConnect(tunnel_t *self, uint8_t tid, bool delay)
 
 static void onStarvedConnectionExpire(idle_item_t *idle_con)
 {
-    LOGW("ReverseClient: a starved connection detected and closed");
     reverse_client_con_state_t *cstate = idle_con->userdata;
     tunnel_t                   *self   = cstate->self;
     reverse_client_state_t     *state  = STATE(self);
+    if (cstate->idle_handle_removed){
+        //this can happen if we are unlucky and 2 events are passed to eventloop in 
+        // a bad order, first connection to peer succeds and also the starvation cb call 
+        // is already in the queue
+        assert(cstate->pair_connected);
+        return;
+    }
 
     if (cstate->handshaked)
     {
+        assert(! cstate->pair_connected);
         if (state->unused_cons[cstate->u->tid] > 0)
         {
             state->unused_cons[cstate->u->tid] -= 1;
         }
+        LOGW("ReverseClient: a idle connection detected and closed");
+    }
+    else
+    {
+        LOGW("ReverseClient: a starved connection detected and closed");
     }
 
     cstate->idle_handle_removed = true;

diff --git a/tunnels/client/reverse/reverse_client.c b/tunnels/client/reverse/reverse_client.c
@@ -116,14 +116,15 @@ static void downStream(tunnel_t *self, context_t *c)
                     state->unused_cons[tid] += 1;
                     LOGI("ReverseClient: connected,    tid: %d unused: %u active: %d", tid, state->unused_cons[tid],
                          atomic_load_explicit(&(state->reverse_cons), memory_order_relaxed));
-                    idle_item_t *con_idle = getIdleItemByHash(tid, state->starved_connections, (hash_t) (ucstate));
-                    if (con_idle != NULL)
-                    {
-                        keepIdleItemForAtleast(state->starved_connections,con_idle,
-                                               kConnectionStarvationTimeOutAfterFirstConfirmation);
-                    }
+
                     if (bufLen(c->payload) > 0)
                     {
+                        idle_item_t *con_idle = getIdleItemByHash(tid, state->starved_connections, (hash_t) (ucstate));
+                        if (con_idle != NULL)
+                        {
+                            keepIdleItemForAtleast(state->starved_connections, con_idle,
+                                                   kConnectionStarvationTimeOutAfterFirstConfirmation);
+                        }
                         continue;
                     }
                     reuseContextBuffer(c);
@@ -174,9 +175,12 @@ static void downStream(tunnel_t *self, context_t *c)
             idle_item_t *con_idle_item =
                 newIdleItem(state->starved_connections, (hash_t) (ucstate), ucstate, onStarvedConnectionExpire,
                             c->line->tid, kConnectionStarvationTimeOut);
-            ucstate->idle_handle_removed = false;
 
-            (void) con_idle_item;
+            if (con_idle_item)
+            {
+                ucstate->idle_handle_removed = false;
+            }
+
             destroyContext(c);
         }
         else