[pacman-dev] Add multi_curl handle to ALPM global context

Message ID 20200309222312.208724-1-anatol.pomozov@gmail.com
State Accepted
Headers show
Series [pacman-dev] Add multi_curl handle to ALPM global context | expand

Commit Message

Anatol Pomozov March 9, 2020, 10:23 p.m. UTC
To be able to run multiple download in parallel efficiently we need to
use curl_multi interface [1]. It introduces a set of APIs over new type
of handler 'CURLM'.

Create CURLM object at the application start and set it to global ALPM
context.

The 'single-download' CURL handle moves to payload struct. A new CURL
handle is created for each payload with intention to be processed by CURLM.

Note that curl_download_internal() is not ported to CURLM interface due
to the fact that the function will go away soon.

[1] https://curl.haxx.se/libcurl/c/libcurl-multi.html
---
 lib/libalpm/alpm.c   | 12 +++++++++---
 lib/libalpm/dload.c  | 17 ++++++-----------
 lib/libalpm/dload.h  |  1 +
 lib/libalpm/handle.c |  5 -----
 lib/libalpm/handle.h |  2 +-
 5 files changed, 17 insertions(+), 20 deletions(-)

Comments

Allan McRae March 17, 2020, 9:17 a.m. UTC | #1
On 10/3/20 8:23 am, Anatol Pomozov wrote:
> To be able to run multiple download in parallel efficiently we need to
> use curl_multi interface [1]. It introduces a set of APIs over new type
> of handler 'CURLM'.
> 
> Create CURLM object at the application start and set it to global ALPM
> context.
> 
> The 'single-download' CURL handle moves to payload struct. A new CURL
> handle is created for each payload with intention to be processed by CURLM.
> 
> Note that curl_download_internal() is not ported to CURLM interface due
> to the fact that the function will go away soon.
> 
> [1] https://curl.haxx.se/libcurl/c/libcurl-multi.html
> ---

Ack - this patch looks fine to me.

A

>  lib/libalpm/alpm.c   | 12 +++++++++---
>  lib/libalpm/dload.c  | 17 ++++++-----------
>  lib/libalpm/dload.h  |  1 +
>  lib/libalpm/handle.c |  5 -----
>  lib/libalpm/handle.h |  2 +-
>  5 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
> index d1265214..34c5b4b2 100644
> --- a/lib/libalpm/alpm.c
> +++ b/lib/libalpm/alpm.c
> @@ -70,6 +70,11 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath,
>  		goto cleanup;
>  	}
>  
> +#ifdef HAVE_LIBCURL
> +	curl_global_init(CURL_GLOBAL_ALL);
> +	myhandle->curlm = curl_multi_init();
> +#endif
> +
>  #ifdef ENABLE_NLS
>  	bindtextdomain("libalpm", LOCALEDIR);
>  #endif
> @@ -104,13 +109,14 @@ int SYMEXPORT alpm_release(alpm_handle_t *myhandle)
>  		ret = -1;
>  	}
>  
> -	_alpm_handle_unlock(myhandle);
> -	_alpm_handle_free(myhandle);
> -
>  #ifdef HAVE_LIBCURL
> +	curl_multi_cleanup(myhandle->curlm);
>  	curl_global_cleanup();
>  #endif
>  
> +	_alpm_handle_unlock(myhandle);
> +	_alpm_handle_free(myhandle);
> +
>  	return ret;
>  }
>  
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index bec3ff5e..1f28c7c2 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -72,15 +72,6 @@ static char *get_fullpath(const char *path, const char *filename,
>  	return filepath;
>  }
>  
> -static CURL *get_libcurl_handle(alpm_handle_t *handle)
> -{
> -	if(!handle->curl) {
> -		curl_global_init(CURL_GLOBAL_SSL);
> -		handle->curl = curl_easy_init();
> -	}
> -	return handle->curl;
> -}
> -
>  enum {
>  	ABORT_SIGINT = 1,
>  	ABORT_OVER_MAXFILESIZE
> @@ -241,7 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t size, size_t nmemb, void *u
>  		}
>  	}
>  
> -	curl_easy_getinfo(payload->handle->curl, CURLINFO_RESPONSE_CODE, &respcode);
> +	curl_easy_getinfo(payload->curl, CURLINFO_RESPONSE_CODE, &respcode);
>  	if(payload->respcode != respcode) {
>  		payload->respcode = respcode;
>  	}
> @@ -377,8 +368,9 @@ static int curl_download_internal(struct dload_payload *payload,
>  	struct sigaction orig_sig_pipe, orig_sig_int;
>  	/* shortcut to our handle within the payload */
>  	alpm_handle_t *handle = payload->handle;
> -	CURL *curl = get_libcurl_handle(handle);
> +	CURL *curl = curl_easy_init();
>  	handle->pm_errno = ALPM_ERR_OK;
> +	payload->curl = curl;
>  
>  	/* make sure these are NULL */
>  	FREE(payload->tempfile_name);
> @@ -594,6 +586,9 @@ cleanup:
>  		unlink(payload->tempfile_name);
>  	}
>  
> +	curl_easy_cleanup(curl);
> +	payload->curl = NULL;
> +
>  	/* restore the old signal handlers */
>  	unmask_signal(SIGINT, &orig_sig_int);
>  	unmask_signal(SIGPIPE, &orig_sig_pipe);
> diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> index 3eb7fbe1..65fcdadb 100644
> --- a/lib/libalpm/dload.h
> +++ b/lib/libalpm/dload.h
> @@ -44,6 +44,7 @@ struct dload_payload {
>  	int trust_remote_name;
>  	int cb_initialized;
>  #ifdef HAVE_LIBCURL
> +	CURL *curl;
>  	CURLcode curlerr;       /* last error produced by curl */
>  #endif
>  };
> diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
> index 8ae08d11..0d4797bf 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -64,11 +64,6 @@ void _alpm_handle_free(alpm_handle_t *handle)
>  		closelog();
>  	}
>  
> -#ifdef HAVE_LIBCURL
> -	/* release curl handle */
> -	curl_easy_cleanup(handle->curl);
> -#endif
> -
>  #ifdef HAVE_LIBGPGME
>  	FREELIST(handle->known_keys);
>  #endif
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index cd7104f9..9fef0fbf 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -59,7 +59,7 @@ struct __alpm_handle_t {
>  
>  #ifdef HAVE_LIBCURL
>  	/* libcurl handle */
> -	CURL *curl;             /* reusable curl_easy handle */
> +	CURLM *curlm;
>  	unsigned short disable_dl_timeout;
>  	unsigned int parallel_downloads; /* number of download streams */
>  #endif
>

Patch

diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index d1265214..34c5b4b2 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -70,6 +70,11 @@  alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath,
 		goto cleanup;
 	}
 
+#ifdef HAVE_LIBCURL
+	curl_global_init(CURL_GLOBAL_ALL);
+	myhandle->curlm = curl_multi_init();
+#endif
+
 #ifdef ENABLE_NLS
 	bindtextdomain("libalpm", LOCALEDIR);
 #endif
@@ -104,13 +109,14 @@  int SYMEXPORT alpm_release(alpm_handle_t *myhandle)
 		ret = -1;
 	}
 
-	_alpm_handle_unlock(myhandle);
-	_alpm_handle_free(myhandle);
-
 #ifdef HAVE_LIBCURL
+	curl_multi_cleanup(myhandle->curlm);
 	curl_global_cleanup();
 #endif
 
+	_alpm_handle_unlock(myhandle);
+	_alpm_handle_free(myhandle);
+
 	return ret;
 }
 
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index bec3ff5e..1f28c7c2 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -72,15 +72,6 @@  static char *get_fullpath(const char *path, const char *filename,
 	return filepath;
 }
 
-static CURL *get_libcurl_handle(alpm_handle_t *handle)
-{
-	if(!handle->curl) {
-		curl_global_init(CURL_GLOBAL_SSL);
-		handle->curl = curl_easy_init();
-	}
-	return handle->curl;
-}
-
 enum {
 	ABORT_SIGINT = 1,
 	ABORT_OVER_MAXFILESIZE
@@ -241,7 +232,7 @@  static size_t dload_parseheader_cb(void *ptr, size_t size, size_t nmemb, void *u
 		}
 	}
 
-	curl_easy_getinfo(payload->handle->curl, CURLINFO_RESPONSE_CODE, &respcode);
+	curl_easy_getinfo(payload->curl, CURLINFO_RESPONSE_CODE, &respcode);
 	if(payload->respcode != respcode) {
 		payload->respcode = respcode;
 	}
@@ -377,8 +368,9 @@  static int curl_download_internal(struct dload_payload *payload,
 	struct sigaction orig_sig_pipe, orig_sig_int;
 	/* shortcut to our handle within the payload */
 	alpm_handle_t *handle = payload->handle;
-	CURL *curl = get_libcurl_handle(handle);
+	CURL *curl = curl_easy_init();
 	handle->pm_errno = ALPM_ERR_OK;
+	payload->curl = curl;
 
 	/* make sure these are NULL */
 	FREE(payload->tempfile_name);
@@ -594,6 +586,9 @@  cleanup:
 		unlink(payload->tempfile_name);
 	}
 
+	curl_easy_cleanup(curl);
+	payload->curl = NULL;
+
 	/* restore the old signal handlers */
 	unmask_signal(SIGINT, &orig_sig_int);
 	unmask_signal(SIGPIPE, &orig_sig_pipe);
diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
index 3eb7fbe1..65fcdadb 100644
--- a/lib/libalpm/dload.h
+++ b/lib/libalpm/dload.h
@@ -44,6 +44,7 @@  struct dload_payload {
 	int trust_remote_name;
 	int cb_initialized;
 #ifdef HAVE_LIBCURL
+	CURL *curl;
 	CURLcode curlerr;       /* last error produced by curl */
 #endif
 };
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index 8ae08d11..0d4797bf 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -64,11 +64,6 @@  void _alpm_handle_free(alpm_handle_t *handle)
 		closelog();
 	}
 
-#ifdef HAVE_LIBCURL
-	/* release curl handle */
-	curl_easy_cleanup(handle->curl);
-#endif
-
 #ifdef HAVE_LIBGPGME
 	FREELIST(handle->known_keys);
 #endif
diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
index cd7104f9..9fef0fbf 100644
--- a/lib/libalpm/handle.h
+++ b/lib/libalpm/handle.h
@@ -59,7 +59,7 @@  struct __alpm_handle_t {
 
 #ifdef HAVE_LIBCURL
 	/* libcurl handle */
-	CURL *curl;             /* reusable curl_easy handle */
+	CURLM *curlm;
 	unsigned short disable_dl_timeout;
 	unsigned int parallel_downloads; /* number of download streams */
 #endif