connectivity: create a multi-handle for all requestslr/conn-check-af

This is dead ugly and works around what probably is a libcurl (7.59) bug:
It shares the resolver cache among easy handles in a multi handle when
it shouldn't:

When we create two easy handles, one for IPv4 and one for IPv6 in a
single mhandle:

  curl_easy_setopt (ehandle6, CURLOPT_RESOLVE,
    "fedoraproject.org:2605:bc80:3010:600:dead:beef:cafe:feda"
    "fedoraproject.org:2604:1580:fe00:0:dead:beef:cafe:fed1")
  curl_easy_setopt (ehandle4, CURLOPT_RESOLVE,
    "fedoraproject.org:8.43.85.67"
    "fedoraproject.org:152.19.134.198")
  curl_multi_add_handle (mhandle, ehandle6);
  curl_multi_add_handle (mhandle, ehandle4);

Both end up connecting to the same (either v4 or v6) address. None of
CURLOPT_FORBID_REUSE(3), CURLOPT_FRESH_CONNECT(3),
CURLOPT_DNS_USE_GLOBAL_CACHE(3) and CURLOPT_DNS_CACHE_TIMEOUT(3) make
any difference. Nor does CURLOPT_DNS_LOCAL_IP[46].

1 files changed, 45 insertions, 54 deletions

diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c
index 3e39960b4d..ce48a6fc4e 100644
--- a/src/nm-connectivity.c
+++ b/src/nm-connectivity.c
@@ -76,6 +76,8 @@ struct _NMConnectivityCheckHandle {
 
 		int ifindex;
 		GCancellable *resolve_cancellable;
+		CURLM *curl_mhandle;
+		guint curl_timer;
 		CURL *curl_ehandle;
 		struct curl_slist *request_headers;
 		struct curl_slist *hosts;
@@ -109,12 +111,6 @@ typedef struct {
 	gboolean enabled;
 	guint interval;
 	NMConfig *config;
-#if WITH_CONCHECK
-	struct {
-		CURLM *curl_mhandle;
-		guint curl_timer;
-	} concheck;
-#endif
 } NMConnectivityPrivate;
 
 struct _NMConnectivity {
@@ -179,8 +175,6 @@ cb_data_complete (NMConnectivityCheckHandle *cb_data,
 
 #if WITH_CONCHECK
 	if (cb_data->concheck.curl_ehandle) {
-		NMConnectivityPrivate *priv;
-
 		/* Contrary to what cURL manual claim it is *not* safe to remove
 		 * the easy handle "at any moment"; specifically it's not safe to
 		 * remove *any* handle from within a libcurl callback. That is
@@ -195,14 +189,15 @@ cb_data_complete (NMConnectivityCheckHandle *cb_data,
 		curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_PRIVATE, NULL);
 		curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_HTTPHEADER, NULL);
 
-		priv = NM_CONNECTIVITY_GET_PRIVATE (self);
-
-		curl_multi_remove_handle (priv->concheck.curl_mhandle, cb_data->concheck.curl_ehandle);
+		curl_multi_remove_handle (cb_data->concheck.curl_mhandle,
+		                          cb_data->concheck.curl_ehandle);
 		curl_easy_cleanup (cb_data->concheck.curl_ehandle);
+		curl_multi_cleanup (cb_data->concheck.curl_mhandle);
 
 		curl_slist_free_all (cb_data->concheck.request_headers);
 		curl_slist_free_all (cb_data->concheck.hosts);
 	}
+	nm_clear_g_source (&cb_data->concheck.curl_timer);
 	nm_clear_g_cancellable (&cb_data->concheck.resolve_cancellable);
 #endif
 
@@ -359,29 +354,27 @@ _con_curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask)
 static gboolean
 _con_curl_timeout_cb (gpointer user_data)
 {
-	gs_unref_object NMConnectivity *self = g_object_ref (NM_CONNECTIVITY (user_data));
-	NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self);
+	NMConnectivityCheckHandle *cb_data = user_data;
 
-	priv->concheck.curl_timer = 0;
-	_con_curl_check_connectivity (priv->concheck.curl_mhandle, CURL_SOCKET_TIMEOUT, 0);
-	_complete_queued (self);
+	cb_data->concheck.curl_timer = 0;
+	_con_curl_check_connectivity (cb_data->concheck.curl_mhandle, CURL_SOCKET_TIMEOUT, 0);
+	_complete_queued (cb_data->self);
 	return G_SOURCE_REMOVE;
 }
 
 static int
 multi_timer_cb (CURLM *multi, long timeout_ms, void *userdata)
 {
-	NMConnectivity *self = NM_CONNECTIVITY (userdata);
-	NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self);
+	NMConnectivityCheckHandle *cb_data = userdata;
 
-	nm_clear_g_source (&priv->concheck.curl_timer);
+	nm_clear_g_source (&cb_data->concheck.curl_timer);
 	if (timeout_ms != -1)
-		priv->concheck.curl_timer = g_timeout_add (timeout_ms, _con_curl_timeout_cb, self);
+		cb_data->concheck.curl_timer = g_timeout_add (timeout_ms, _con_curl_timeout_cb, cb_data);
 	return 0;
 }
 
 typedef struct {
-	NMConnectivity *self;
+	NMConnectivityCheckHandle *cb_data;
 	GIOChannel *ch;
 
 	/* this is a very simplistic weak-pointer. If ConCurlSockData gets
@@ -398,8 +391,7 @@ static gboolean
 _con_curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_data)
 {
 	ConCurlSockData *fdp = user_data;
-	gs_unref_object NMConnectivity *self = g_object_ref (fdp->self);
-	NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self);
+	NMConnectivityCheckHandle *cb_data = fdp->cb_data;
 	int fd = g_io_channel_unix_get_fd (ch);
 	int action = 0;
 	gboolean fdp_destroyed = FALSE;
@@ -415,7 +407,7 @@ _con_curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_
 	nm_assert (!fdp->destroy_notify);
 	fdp->destroy_notify = &fdp_destroyed;
 
-	success = _con_curl_check_connectivity (priv->concheck.curl_mhandle, fd, action);
+	success = _con_curl_check_connectivity (cb_data->concheck.curl_mhandle, fd, action);
 
 	if (fdp_destroyed) {
 		/* hups. fdp got invalidated during _con_curl_check_connectivity(). That's fine,
@@ -427,7 +419,7 @@ _con_curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_
 			fdp->ev = 0;
 	}
 
-	_complete_queued (self);
+	_complete_queued (cb_data->self);
 
 	return success ? G_SOURCE_CONTINUE : G_SOURCE_REMOVE;
 }
@@ -435,8 +427,7 @@ _con_curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_
 static int
 multi_socket_cb (CURL *e_handle, curl_socket_t fd, int what, void *userdata, void *socketp)
 {
-	NMConnectivity *self = NM_CONNECTIVITY (userdata);
-	NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self);
+	NMConnectivityCheckHandle *cb_data = userdata;
 	ConCurlSockData *fdp = socketp;
 	GIOCondition condition = 0;
 
@@ -446,7 +437,7 @@ multi_socket_cb (CURL *e_handle, curl_socket_t fd, int what, void *userdata, voi
 		if (fdp) {
 			if (fdp->destroy_notify)
 				*fdp->destroy_notify = TRUE;
-			curl_multi_assign (priv->concheck.curl_mhandle, fd, NULL);
+			curl_multi_assign (cb_data->concheck.curl_mhandle, fd, NULL);
 			nm_clear_g_source (&fdp->ev);
 			g_io_channel_unref (fdp->ch);
 			g_slice_free (ConCurlSockData, fdp);
@@ -454,9 +445,9 @@ multi_socket_cb (CURL *e_handle, curl_socket_t fd, int what, void *userdata, voi
 	} else {
 		if (!fdp) {
 			fdp = g_slice_new0 (ConCurlSockData);
-			fdp->self = self;
+			fdp->cb_data = cb_data;
 			fdp->ch = g_io_channel_unix_new (fd);
-			curl_multi_assign (priv->concheck.curl_mhandle, fd, fdp);
+			curl_multi_assign (cb_data->concheck.curl_mhandle, fd, fdp);
 		} else
 			nm_clear_g_source (&fdp->ev);
 
@@ -573,15 +564,35 @@ static void
 do_curl_request (NMConnectivityCheckHandle *cb_data)
 {
 	NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (cb_data->self);
+	CURLM *mhandle;
 	CURL *ehandle;
 	long resolve;
 
+	mhandle = curl_multi_init ();
+	if (!mhandle) {
+		cb_data_complete (cb_data, NM_CONNECTIVITY_ERROR, "curl error");
+		return;
+	}
+
 	ehandle = curl_easy_init ();
 	if (!ehandle) {
-		cb_data_free (cb_data, NM_CONNECTIVITY_ERROR, NULL, "curl error");
+		curl_multi_cleanup (mhandle);
+		cb_data_complete (cb_data, NM_CONNECTIVITY_ERROR, "curl error");
 		return;
 	}
 
+	cb_data->concheck.response = g_strdup (priv->response);
+	cb_data->concheck.curl_mhandle = mhandle;
+	cb_data->concheck.curl_ehandle = ehandle;
+	cb_data->concheck.request_headers = curl_slist_append (NULL, "Connection: close");
+	cb_data->timeout_id = g_timeout_add_seconds (20, _timeout_cb, cb_data);
+
+	curl_multi_setopt (mhandle, CURLMOPT_SOCKETFUNCTION, multi_socket_cb);
+	curl_multi_setopt (mhandle, CURLMOPT_SOCKETDATA, cb_data);
+	curl_multi_setopt (mhandle, CURLMOPT_TIMERFUNCTION, multi_timer_cb);
+	curl_multi_setopt (mhandle, CURLMOPT_TIMERDATA, cb_data);
+	curl_multi_setopt (mhandle, CURLOPT_VERBOSE, 1);
+
 	switch (cb_data->addr_family) {
 	case AF_INET:
 		resolve = CURL_IPRESOLVE_V4;
@@ -597,11 +608,6 @@ do_curl_request (NMConnectivityCheckHandle *cb_data)
 		g_warn_if_reached ();
 	}
 
-	cb_data->concheck.response = g_strdup (priv->response);
-	cb_data->concheck.curl_ehandle = ehandle;
-	cb_data->concheck.request_headers = curl_slist_append (NULL, "Connection: close");
-	cb_data->timeout_id = g_timeout_add_seconds (20, _timeout_cb, cb_data);
-
 	curl_easy_setopt (ehandle, CURLOPT_URL, priv->uri);
 	curl_easy_setopt (ehandle, CURLOPT_WRITEFUNCTION, easy_write_cb);
 	curl_easy_setopt (ehandle, CURLOPT_WRITEDATA, cb_data);
@@ -613,7 +619,7 @@ do_curl_request (NMConnectivityCheckHandle *cb_data)
 	curl_easy_setopt (ehandle, CURLOPT_RESOLVE, cb_data->concheck.hosts);
 	curl_easy_setopt (ehandle, CURLOPT_IPRESOLVE, resolve);
 
-	curl_multi_add_handle (priv->concheck.curl_mhandle, ehandle);
+	curl_multi_add_handle (mhandle, ehandle);
 }
 
 static void
@@ -899,11 +905,8 @@ update_config (NMConnectivity *self, NMConfigData *config_data)
 
 	enabled = FALSE;
 #if WITH_CONCHECK
-	/* connectivity checking also requires a valid URI, interval and
-	 * curl_mhandle */
 	if (   priv->uri
-	    && priv->interval
-	    && priv->concheck.curl_mhandle)
+	    && priv->interval)
 		enabled = nm_config_data_get_connectivity_enabled (config_data);
 #endif
 
@@ -957,18 +960,9 @@ nm_connectivity_init (NMConnectivity *self)
 
 #if WITH_CONCHECK
 	ret = curl_global_init (CURL_GLOBAL_ALL);
-	if (ret == CURLE_OK)
-		priv->concheck.curl_mhandle = curl_multi_init ();
-
-	if (!priv->concheck.curl_mhandle) {
+	if (!ret == CURLE_OK) {
 		 _LOGE ("unable to init cURL, connectivity check will not work: (%d) %s",
 		        ret, curl_easy_strerror (ret));
-	} else {
-		curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_SOCKETFUNCTION, multi_socket_cb);
-		curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_SOCKETDATA, self);
-		curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_TIMERFUNCTION, multi_timer_cb);
-		curl_multi_setopt (priv->concheck.curl_mhandle, CURLMOPT_TIMERDATA, self);
-		curl_multi_setopt (priv->concheck.curl_mhandle, CURLOPT_VERBOSE, 1);
 	}
 #endif
 
@@ -995,9 +989,6 @@ dispose (GObject *object)
 	g_clear_pointer (&priv->response, g_free);
 
 #if WITH_CONCHECK
-	nm_clear_g_source (&priv->concheck.curl_timer);
-
-	curl_multi_cleanup (priv->concheck.curl_mhandle);
 	curl_global_cleanup ();
 #endif