multi: update timer unconditionally in multi_remove_handle

When removing an easy handle from a multi, there was an optimization
to update the timer only when the removed handle had any timers.

With the introduction of the "dirty" bitset, easy handles can now cause
a timeout of 0 to be set without having anything in their timer list.
Removing such a handle needs to update the timer now always, so that
it may get cleared when there is nothing more to wait for.

The previous "not clearing a 0 timer" should not have any effect on
application's logic. Without clearing, the timer will fire and then
adjust itself to the proper value. But it would cause one more timer
fire than necessary.

Reported-by: Jan Macku
Fixes https://github.com/curl/curl/issues/20498
Closes https://github.com/curl/curl/pull/20502

diff --git a/lib/multi.c b/lib/multi.c
index 99d1adcb51adb88096cfbcf37cd528f4ac053bf2..025a2a7acb418ce195a0452b9728481e4cbec2d5 100644 (file)
--- a/lib/multi.c
+++ b/lib/multi.c
@@ -753,7 +753,6 @@ CURLMcode curl_multi_remove_handle(CURLM *m, CURL *d)
   bool premature;
   struct Curl_llist_node *e;
   CURLMcode mresult;
-  bool removed_timer = FALSE;
   uint32_t mid;
 
   /* First, make some basic checks that the CURLM handle is a good handle */
@@ -808,7 +807,7 @@ CURLMcode curl_multi_remove_handle(CURLM *m, CURL *d)
   /* The timer must be shut down before data->multi is set to NULL, else the
      timenode will remain in the splay tree after curl_easy_cleanup is
      called. Do it after multi_done() in case that sets another time! */
-  removed_timer = Curl_expire_clear(data);
+  Curl_expire_clear(data);
 
   /* If in `msgsent`, it was deducted from `multi->xfers_alive` already. */
   if(!Curl_uint32_bset_contains(&multi->msgsent, data->mid))
@@ -881,11 +880,9 @@ CURLMcode curl_multi_remove_handle(CURLM *m, CURL *d)
      We do not touch the easy handle here! */
   process_pending_handles(multi);
 
-  if(removed_timer) {
-    mresult = Curl_update_timer(multi);
-    if(mresult)
-      return mresult;
-  }
+  mresult = Curl_update_timer(multi);
+  if(mresult)
+    return mresult;
 
   CURL_TRC_M(data, "removed from multi, mid=%u, running=%u, total=%u",
              mid, Curl_multi_xfers_running(multi),
@@ -3604,7 +3601,7 @@ void Curl_expire_done(struct Curl_easy *data, expire_id eid)
  *
  * Clear ALL timeout values for this handle.
  */
-bool Curl_expire_clear(struct Curl_easy *data)
+void Curl_expire_clear(struct Curl_easy *data)
 {
   struct Curl_multi *multi = data->multi;
   struct curltime *nowp = &data->state.expiretime;
@@ -3612,7 +3609,7 @@ bool Curl_expire_clear(struct Curl_easy *data)
   /* this is only interesting while there is still an associated multi struct
      remaining! */
   if(!multi)
-    return FALSE;
+    return;
 
   if(nowp->tv_sec || nowp->tv_usec) {
     /* Since this is an cleared time, we must remove the previous entry from
@@ -3632,9 +3629,7 @@ bool Curl_expire_clear(struct Curl_easy *data)
       CURL_TRC_M(data, "[TIMEOUT] all cleared");
     nowp->tv_sec = 0;
     nowp->tv_usec = 0;
-    return TRUE;
   }
-  return FALSE;
 }
 
 CURLMcode curl_multi_assign(CURLM *m, curl_socket_t s,

diff --git a/lib/multiif.h b/lib/multiif.h
index 52f799b7c0b5784eb00d928ebfabbe2ae9642f9c..0252b2dfd978b5626cc3f8e1e196440ab735c6f6 100644 (file)
--- a/lib/multiif.h
+++ b/lib/multiif.h
@@ -30,7 +30,7 @@
 void Curl_expire(struct Curl_easy *data, timediff_t milli, expire_id);
 void Curl_expire_ex(struct Curl_easy *data,
                     timediff_t milli, expire_id id);
-bool Curl_expire_clear(struct Curl_easy *data);
+void Curl_expire_clear(struct Curl_easy *data);
 void Curl_expire_done(struct Curl_easy *data, expire_id id);
 CURLMcode Curl_update_timer(struct Curl_multi *multi) WARN_UNUSED_RESULT;
 void Curl_attach_connection(struct Curl_easy *data,