Share connection between curl requests #170
Conversation
|
I have some concerns about sharing the connection (CURL_LOCK_DATA_CONNECT) between the mainloop (poller) and the download thread. The documentation states: "It is not supported to share connections between multiple concurrent threads." I did place locking on the shared data including CURL_LOCK_DATA_CONNECT though. |
|
I think inter-thread connection sharing as it's implemented here should be okay. Maybe we should add |
Added. |
Bastian-Krause
force-pushed
the
master-share
branch
from
e84e3ec to
0f76f69
Compare
TSAN works by inserting instrumentation into compiled code to detect race conditions. We use glib from Debian in CI, meaning those libraries are not built with TSAN instrumentation, so memory accesses cannot be tracked. This leads to false positives. So suggesting to enable the sanitizer was not a good idea after all. I'll drop the commit. |
Bastian-Krause
force-pushed
the
master-share
branch
from
0f76f69 to
7a8e42e
Compare
Bastian-Krause
force-pushed
the
master-share
branch
from
7a8e42e to
33bd37a
Compare
|
I've changed the PR slightly:
|
Using a curl_share between curl requests allows sharing an open connection and re-use cached DNS, PSL and TLS session id. This change allows performing multiple requests without having to do re-perform the (full) TLS handshake. For reference, on a stm32mp151c with OPTEE + pkcs11 TA a full TLS handshake takes ~8 seconds. Mostly due to small pager pool (internal sram) available for OPTEE. With this change a mTLS curl request take around 60ms after the initial connection has been established. Signed-off-by: Robin van der Gracht <[email protected]> Signed-off-by: Bastian Krause <[email protected]>
Bastian-Krause
force-pushed
the
master-share
branch
from
33bd37a to
56feb22
Compare
| g_mutex_init(&curl_share_locks[i]); | ||
|
|
||
| curl_share = curl_share_init(); | ||
| g_return_if_fail(curl_share); |
There was a problem hiding this comment.
We use g_return_*() methods for input verification only. Thus better either ignore it (since all potential errors for curl_share_setopt() are ignored so far anyway) or use g_assert_nonnull().
| @@ -256,6 +274,24 @@ static void set_default_curl_opts(CURL *curl) | |||
| curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, hawkbit_config->connect_timeout); | |||
| curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, hawkbit_config->ssl_verify ? 1L : 0L); | |||
| curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, hawkbit_config->ssl_verify ? 1L : 0L); | |||
|
|
|||
| if (g_once_init_enter(&curl_share_initialized)) { | |||
| for (gint i = 0; i <= CURL_LOCK_DATA_LAST; i++) | |||
There was a problem hiding this comment.
Maybe this deserves a short comment explaining that curl shopt requires a different lock for each data type (that is initialized here)?
| @@ -85,6 +89,20 @@ GQuark rhu_hawkbit_client_http_error_quark(void) | |||
| return g_quark_from_static_string("rhu_hawkbit_client_http_error_quark"); | |||
| } | |||
|
|
|||
| void curl_share_lock(CURL *handle, curl_lock_data data, curl_lock_access access, void *clientp) | |||
| { | |||
| g_assert_cmpint(data, <=, CURL_LOCK_DATA_LAST); | |||
There was a problem hiding this comment.
Here, g_return_* might fit better as it's an input parameter validation.
Using a curl_share between curl requests allows sharing an open connection and re-use cached DNS, PSL and TLS session id.
This change allows performing multiple requests without having to do re-perform the (full) TLS handshake.
For reference, on a stm32mp151c with OPTEE + pkcs11 TA a full TLS handshake takes ~8 seconds. Mostly due to small pager pool (internal sram) available for OPTEE.
With this change a mTLS curl request take around 60ms after the initial connection has been established.