godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.13k stars 21.19k forks source link

Implement non blocking SSL handshake #18973

Closed Keetz closed 6 years ago

Keetz commented 6 years ago

Godot version: 3.0.2 stable and 3.0.3 rc2

Issue description: When making a https request it blocks everything, making everything freeze for the duration of the request. This causes the editor to lag, it causes the game to lag, it just blocks the main thread it seems.

I found that the problem is a while loop in 'stream_peer_mbed_tls', more specific this loop: https://github.com/godotengine/godot/blob/1710582473330dc6e7758953e3378187b5e3f226/modules/mbedtls/stream_peer_mbed_tls.cpp#L115-L122

When this loop is reached, everything stops. Notice! It is a bit above me to say what exactly is going on, and what causes this, so might be somewhere else than that exact loop, I just found that to be the place by debugging a bit.

Steps to reproduce: I modified the http request example from the docs a bit so: Download the attached project, run it and press the left arrow key to make a https request. Watch the animated godot icon lag as the request is happening.

Also try and do a http request instead, just to see that it doesn't happen. As simple as changing: err = http.connect_to_host("https://php.net") # Connect to host/port To: err = http.connect_to_host("http://php.net") # Connect to host/port

Minimal reproduction project: HTTPSClientTest.tar.gz

Keetz commented 6 years ago

@Faless if you can point me in the direction of better debugging and fixing this I would gladly look into it myself, I am just stuck right here.

Keetz commented 6 years ago

Side note:

You might notice I print the time elapsed from start to finish. I did this because the request seems to take 4-5 times longer compared to, for example a RESTClient addon for Firefox or Chrome.

This might be because of the same while loop just running way too many iterations, but not sure.

Faless commented 6 years ago

@Keetz will look into it, few notes:

I will try with master branch and stable and try to see what is going on.

Faless commented 6 years ago

As said above, non blocking SSL handshake is not implemented yet in Godot, so if you want it not to block you have to thread it for now. I'll try to find some spare time to implement it, though it is not trivial.

I did this because the request seems to take 4-5 times longer compared to, for example a RESTClient addon for Firefox or Chrome.

That might be related to the way HTTPClient handles the response though, and the fact that you only poll once every idle_frame, which might add an extra delay between one poll and the other, especially on big requests and when you have FPS drops (not the case for https://php.net though).

Keetz commented 6 years ago

@Faless cheers!

The threading option is a bit of a problem on WebAssembly as it doesn't support threading yet. So implementing non blocking SSL handshake sounds awesome!

If you are running on 3.0.x the mbedtls patch is not merged

I was testing this on master, my bad. But as you pointed out the problem is the there with OpenSSL too.

Faless commented 6 years ago

The threading option is a bit of a problem on WebAssembly as it doesn't support threading yet

WebAssembly uses native browser functionalities for networking, it shouldn't block there as mbedTLS (or OpenSSL) do not get compiled at all. Specifically HTTPClient in HTML5 export is done via native XMLHttpRequests. Try exporting the project, you shouldn't experience those freezes in the HTML5/WebAssembly version.

Keetz commented 6 years ago

@Faless ah okay, thanks for clarifying! You are right, no freezes in WebAssembly version.

Faless commented 6 years ago

@Keetz , can you try out this branch: https://github.com/Faless/godot/tree/ssl_server . It includes commits to make the handshake non blocking. HTTPClient does that for you automatically, while if you use StreamPeerSSL manually, you must set StreamPeerSSL.blocking_handshake = false before connecting. There is an additional StreamPeerSSL.STATUS_HANDSHAKING to signal a yet to finish handshake (you cannot put_data or get_data in that state).

Keetz commented 6 years ago

@Faless works like a charm! I didn't test the StreamPeerSSL though. But I gave the HTTPClient a good test. Both a small demo project and one of our full scale projects.

Faless commented 6 years ago

Thanks! I'll PR it later today :+1: