[no squash] Get rid of the last remaining sync. HTTP requests on the main thread #14649
+150
−51
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rubenwardy
reviewed
May 13, 2024
grorp
force-pushed
the
no-sync-http-main-thread
branch
4 times, most recently
from
b13197b
to
d89cd5b
Compare
Any sync. HTTP request on the main thread is a bug, don't allow introducing one again.
grorp
force-pushed
the
no-sync-http-main-thread
branch
from
d89cd5b
to
6245073
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Building upon the work in #13551 and #14412, this PR aims to make sure that there will never be ANRs related to HTTP requests again.
Commit 1 moves the last remaining HTTP request from the main thread to
core.handle_async. To reduce the amount of changes to existing code and to avoid callback hell, it uses coroutines.The HTTP request in question is the one that fetches a dependency list from CDB before attempting to install a package. While it's usually very fast, it can take long if you have a bad internet connection or if it times out.
Commit 2 removes the ability to execute synchronous HTTP requests on the main thread from
httpfetch.cpp. This doesn't break backwards compatibility becausefetch_syncis only available in the mainmenu. Since any synchronous HTTP request on the main thread is a bug, we should avoid introducing one again in the future.To do
This PR is a Ready for Review.
httpfetch.cpp.There is probably more to remove, but I fear that I don't know the code well enough to avoid accidentally breaking things.
How to test
Verify that Minetest doesn't freeze for a short while anymore after you press the install button on a package.
Try to install lots of different packages from CDB. Verify that dependencies are resolved correctly and that packages are installed correctly.