Tricky.

The commit you refer to fixed a real bug. Before that, easy handles could be left in the list splay tree for timeouts even though the transfer had indeed ended and should've been removed. But fixing that bug caused some subtle changes in what curl_multi_timeout() would return. We even had mistakes around the return code in our own test cases so I'm certainly not blaming anyone for getting things wrong and libcurl did after all return this "wrong" value like this for a very long time.

curl_multi_timeout() could still return -1 even before this fix, for example before any handles were added and similar. It was just more rare.

Timeout value 0 means run again straight away and that was clearly not the right thing for that situation.

However, I don't see the problem you mention in multi-app.c? This example doesn't do the 100ms sleep based on the timeout value, it does it based on the maxfd returned from curl_multi_fdset() and that didn't change in this commit did it? The example should probably rather do the sleep based on the curl_multi_timeout() return value to make it behave better...

Indeed. The code of examples/multi-app.c and librepo/downloader.c has diverged over the time and the example now suffers from the mentioned performance penalty independently of commit caa4efb. But this is just a coincidence -- the performance regression in that example has already been introduced by commit cb13fad (and on Windows already via a607f8a).

I changed the example to download file:///tmp/empty0 and file:///tmp/empty1 and it was wastefully sleeping 100ms after a successful download. Do you think it is appropriate?

This could be fixed in the example by using a while loop instead of do/while loop (perhaps together with adding a check for the return value of curl_multi_perform()):

--- a/docs/examples/multi-app.c
+++ b/docs/examples/multi-app.c
@@ -74,7 +74,7 @@ int main(void)
   /* we start some action by calling perform right away */
   curl_multi_perform(multi_handle, &still_running);

-  do {
+  while(still_running) {
     struct timeval timeout;
     int rc; /* select() return code */
     CURLMcode mc; /* curl_multi_fdset() return code */
@@ -142,7 +142,7 @@ int main(void)
       curl_multi_perform(multi_handle, &still_running);
       break;
     }
-  } while(still_running);
+  }

   /* See how the transfers went */
   while((msg = curl_multi_info_read(multi_handle, &msgs_left))) {

However, people often base their code on the examples distributed with libcurl. So the mentioned change might have broken more libcurl-based software than just librepo.

Actually, it was commit acefdd0 what landed in the master branch (I was referring to an obsolete PR commit by mistake).

This could be fixed in the example by using a while loop instead of do/while loop

We really should do that fix, yes. As a matter of fact I think it would make sense to take this as an opportunity and go through the examples with a comb and make sure that they actually are using the API the way we want them to and the way the API is documented to work... It is very unfortunate when our own examples mislead users.

I am not sure about http2-serverpush.c -- using a while loop instead of do/while loop would break the logic I am afraid. Should we just skip select/sleep in case the first call to curl_multi_perform() returns still_running == 0?

Would it be fair to say that this issue just affects libcurl users that are using libcurl as their "main loop", as opposed to integrating with an external mainloop (e.g. glib)?

librepo does the former, where as e.g. libostree does the latter.

This is a documentation issue. Some of the examples distributed with libcurl used to use its API in a wrong way. The code of librepo was based on those examples, which triggered a problem later on. The code you refer to does not seem to be based on the same examples, so it is unrelated to this issue.

Are you experiencing any performance regression in libostree after upgrade to libcurl-7.61.1?