Skip to content

aria2: increase curl timeout - #1282

Open
rmelotte wants to merge 1 commit into
kiwix:mainfrom
rmelotte:hotfix/increase-aria2-curl-timeout
Open

aria2: increase curl timeout#1282
rmelotte wants to merge 1 commit into
kiwix:mainfrom
rmelotte:hotfix/increase-aria2-curl-timeout

Conversation

@rmelotte

Copy link
Copy Markdown

The starting of aria2c is rather fragile as it depends on quite short fixed timeouts, and because kiwix-desktop cannot recover when it fails.

When this happens, it leads to "Cannot connect to aria2c rpc" when launching kiwix-desktop.

The waiting time has already been increased in the past to try to make it more robust:
#1169

However, curl also has its own internal timeout that is currently shorter than MAX_WAITING_TIME_SECONDS. As a result when aria2 is a bit slow to start (on older systems for example), curl currently times out after 100ms, but we wait for 1 second regardless until we report the issue. The resulting curl error in that case is: "Connection timed out after 100 milliseconds"

As a quick fix to avoid having to change too much of the implementation, just increase curl's timeout to match MAX_WAITING_TIME_SECONDS.

The starting of aria2c is rather fragile as it depends on quite short fixed
timeouts, and because kiwix-desktop cannot recover when it fails.

When this happens, it leads to "Cannot connect to aria2c rpc" when
launching kiwix-desktop.

The waiting time has already been increased in the past to try to make
it more robust:
kiwix#1169

However, curl also has its own internal timeout that is currently
shorter than MAX_WAITING_TIME_SECONDS. As a result when aria2 is a bit
slow to start (on older systems for example), curl currently times out
after 100ms, but we wait for 1 second regardless until we report the
issue. The resulting curl error in that case is: "Connection timed out
after 100 milliseconds"

As a quick fix to avoid having to change too much of the
implementation, just increase curl's timeout to match
MAX_WAITING_TIME_SECONDS.

Signed-off-by: Raphaël Mélotte <raphael.melotte@mind.be>
@kelson42
kelson42 requested a review from veloman-yunkan March 12, 2026 00:54
@kelson42 kelson42 added this to the 15.0.0 milestone Mar 12, 2026
@kelson42

kelson42 commented Mar 12, 2026

Copy link
Copy Markdown
Collaborator

I would strongly recommend to use one unique const for all places where the timeout should be set. Otherwise, next time we have to deal with that, we will forget to change half of the things.

@veloman-yunkan veloman-yunkan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your understanding of the code is wrong. Curl requests (with a short timeout) for checking whether aria2c is running are repeated in a loop until the longer timeout is exceeded. The short timeout doesn't hurt as long as aria2c isn't slow to an extent that it, once up and running, fails to process the request within that duration.

@rmelotte

Copy link
Copy Markdown
Author

Thanks a lot for the review!

Looking at it now, I've no clue how I arrived at the conclusion that curl ran only once, but it's clearly completely wrong...

But then since the current change fixes the issue in my case (I'm running it on a 4-cores x86 Desktop from 2012 so it's old, but not that slow either), it must mean even the last call times out after 100ms.

Would increasing both timeouts (for example the curl timeout to 1000ms and the global timeout to 5 seconds) be acceptable to you, to accommodate slower systems?

@veloman-yunkan

Copy link
Copy Markdown
Collaborator

@rmelotte Your change can be equivalent (under certain conditions) to increasing the global timeout to 2seconds.

I am not in favor of discussing optimal values for hardcoded parameters. I'd rather make this setting user-controllable. @kelson42 What do you think?

@kelson42

Copy link
Copy Markdown
Collaborator

Do we have a valid scenario for which current timeout is not enough? If "yes", what would be the right default values to fix it? What would be the other negative impact of such a value?

To my understanding, I see no real reason to not increase the timeout... if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants