On 2/22/23 16:01, Richard W.M. Jones wrote:
Using the libcurl share interface we can share data between the
separate curl easy handles in the pool. For more about this see:
https://curl.se/libcurl/c/CURLSHOPT_SHARE.html
https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b
https://everything.curl.dev/libcurl/sharing
---
plugins/curl/curldefs.h | 3 +-
plugins/curl/curl.c | 4 ++-
plugins/curl/pool.c | 75 +++++++++++++++++++++++++++++++++++++++--
3 files changed, 78 insertions(+), 4 deletions(-)
diff --git a/plugins/curl/curldefs.h b/plugins/curl/curldefs.h
index c2a3432fc..d614379d0 100644
--- a/plugins/curl/curldefs.h
+++ b/plugins/curl/curldefs.h
@@ -117,9 +117,10 @@ struct curl_handle {
};
/* pool.c */
+extern void load_pool (void);
+extern void unload_pool (void);
extern struct curl_handle *get_handle (void);
extern void put_handle (struct curl_handle *ch);
-extern void free_all_handles (void);
/* scripts.c */
extern int do_scripts (struct curl_handle *ch);
diff --git a/plugins/curl/curl.c b/plugins/curl/curl.c
index b5927b5b4..b8624a6f8 100644
--- a/plugins/curl/curl.c
+++ b/plugins/curl/curl.c
@@ -101,6 +101,8 @@ curl_load (void)
nbdkit_error ("libcurl initialization failed: %d", (int) r);
exit (EXIT_FAILURE);
}
+
+ load_pool ();
}
static void
@@ -112,7 +114,7 @@ curl_unload (void)
free (password);
free (proxy_password);
scripts_unload ();
- free_all_handles ();
+ unload_pool ();
curl_global_cleanup ();
}
diff --git a/plugins/curl/pool.c b/plugins/curl/pool.c
index b6f4f8e5f..536123388 100644
--- a/plugins/curl/pool.c
+++ b/plugins/curl/pool.c
@@ -52,6 +52,7 @@
#include <nbdkit-plugin.h>
+#include "array-size.h"
#include "ascii-ctype.h"
#include "ascii-string.h"
#include "cleanup.h"
@@ -73,6 +74,18 @@ static int debug_cb (CURL *handle, curl_infotype type,
static size_t header_cb (void *ptr, size_t size, size_t nmemb, void *opaque);
static size_t write_cb (char *ptr, size_t size, size_t nmemb, void *opaque);
static size_t read_cb (void *ptr, size_t size, size_t nmemb, void *opaque);
+static void lock_cb (CURL *handle, curl_lock_data data,
+ curl_lock_access access, void *userptr);
+static void unlock_cb (CURL *handle, curl_lock_data data,
+ void *userptr);
+
+/* These locks protect access to the curl share data. See:
+ *
https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b
+ */
+static pthread_rwlock_t lockarray[CURL_LOCK_DATA_LAST];
+
+/* Curl share data. */
+static CURLSH *share;
/* This lock protects access to the curl_handles vector below. */
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
@@ -90,18 +103,46 @@ static curl_handle_list curl_handles = empty_vector;
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
static size_t in_use = 0, waiting = 0;
-/* Close and free all handles in the pool. */
+/* Initialize pool structures. */
void
-free_all_handles (void)
+load_pool (void)
{
size_t i;
+ for (i = 0; i < ARRAY_SIZE (lockarray); ++i)
+ pthread_rwlock_init (&lockarray[i], NULL);
error checking missing
+
+ share = curl_share_init ();
+ if (share == NULL) {
+ nbdkit_error ("curl_share_init: %m");
+ exit (EXIT_FAILURE);
+ }
+ curl_share_setopt (share, CURLSHOPT_LOCKFUNC, lock_cb);
+ curl_share_setopt (share, CURLSHOPT_UNLOCKFUNC, unlock_cb);
+ curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT);
+ curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE);
+ curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS);
+ curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION);
error checking missing
+}
+
+void
+unload_pool (void)
+{
+ size_t i;
+
+ /* Close and free all handles in the pool. */
nbdkit_debug ("free_all_handles: number of curl handles allocated: %zu",
curl_handles.len);
for (i = 0; i < curl_handles.len; ++i)
free_handle (curl_handles.ptr[i]);
curl_handle_list_reset (&curl_handles);
+
+ /* Free share data. */
+ curl_share_cleanup (share);
+
+ for (i = 0; i < ARRAY_SIZE (lockarray); ++i)
+ pthread_rwlock_destroy (&lockarray[i]);
}
/* Get a handle from the pool.
@@ -221,6 +262,9 @@ allocate_handle (void)
goto err;
}
+ /* Share settings with other handles. */
+ curl_easy_setopt (ch->c, CURLOPT_SHARE, share);
+
/* Various options we always set.
*
* NB: Both here and below constants must be explicitly long because
@@ -519,3 +563,30 @@ free_handle (struct curl_handle *ch)
curl_slist_free_all (ch->headers_copy);
free (ch);
}
+
+static void
+lock_cb (CURL *handle, curl_lock_data data, curl_lock_access access,
+ void *userptr)
+{
+ assert (data < ARRAY_SIZE (lockarray));
+
+ switch (access) {
+ case CURL_LOCK_ACCESS_SHARED:
+ pthread_rwlock_rdlock (&lockarray[data]);
+ break;
+ case CURL_LOCK_ACCESS_SINGLE:
+ pthread_rwlock_wrlock (&lockarray[data]);
+ break;
we could at least assert the return values
+ default:
+ /* CURL_LOCK_ACCESS_NONE is never used in the current libcurl code. */
+ abort ();
+ }
+}
+
+static void
+unlock_cb (CURL *handle, curl_lock_data data, void *userptr)
+{
+ assert (data < ARRAY_SIZE (lockarray));
+
+ pthread_rwlock_unlock (&lockarray[data]);
same here
+}
Seems sane to me in general, except for the fact that the documentation
at <
https://curl.se/libcurl/c/CURLSHOPT_SHARE.html> writes:
"""
CURL_LOCK_DATA_CONNECT
Put the connection cache in the share object and make all easy handles
using this share object share the connection cache.
Note that due to a known bug, it is not safe to share connections this
way between multiple concurrent threads. [...]
"""
Ugh, what? If it's not safe to share the connection cache between
threads, then CURL_LOCK_DATA_CONNECT is effectively unusable, and no
connection pooling can be done. How does that *not* make this whole curl
facility useless?
The facility in general looks super weird; what sense does it make *not*
to share some particular CURL_LOCK_DATA_xxx?
Laszlo