diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index 51a6970e32..3b779677da 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -195,7 +195,7 @@ cb_data_free (NMConnectivityCheckHandle *cb_data, * the easy handle "at any moment"; specifically not from the * write function. Thus here we just dissociate the cb_data from * the easy handle and the easy handle will be cleaned up when the - * message goes to CURLMSG_DONE in curl_check_connectivity(). */ + * message goes to CURLMSG_DONE in _con_curl_check_connectivity(). */ curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_WRITEFUNCTION, NULL); curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_WRITEDATA, NULL); curl_easy_setopt (cb_data->concheck.curl_ehandle, CURLOPT_HEADERFUNCTION, NULL); @@ -234,8 +234,8 @@ _check_handle_get_response (NMConnectivityCheckHandle *cb_data) return cb_data->concheck.response ?: NM_CONFIG_DEFAULT_CONNECTIVITY_RESPONSE; } -static void -curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) +static gboolean +_con_curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) { NMConnectivityCheckHandle *cb_data; CURLMsg *msg; @@ -244,10 +244,13 @@ curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) long response_code; CURLMcode ret; int running_handles; + gboolean success = TRUE; ret = curl_multi_socket_action (mhandle, sockfd, ev_bitmask, &running_handles); - if (ret != CURLM_OK) + if (ret != CURLM_OK) { _LOGD ("connectivity check failed: %d", ret); + success = FALSE; + } while ((msg = curl_multi_info_read (mhandle, &m_left))) { @@ -258,6 +261,7 @@ curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) eret = curl_easy_getinfo (msg->easy_handle, CURLINFO_PRIVATE, (char **) &cb_data); if (eret != CURLE_OK) { _LOGD ("curl cannot extract cb_data for easy handle, skipping msg"); + success = FALSE; continue; } @@ -286,16 +290,22 @@ curl_check_connectivity (CURLM *mhandle, int sockfd, int ev_bitmask) "unexpected short response"); } } + + /* if we return a failure, we don't know what went wrong. It's likely serious, because + * a failure here is not expected. Return FALSE, so that we stop polling the file descriptor. + * Worst case, this leaves the pending connectivity check unhandled, until our regular + * time-out kicks in. */ + return success; } static gboolean -curl_timeout_cb (gpointer user_data) +_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); priv->concheck.curl_timer = 0; - curl_check_connectivity (priv->concheck.curl_mhandle, CURL_SOCKET_TIMEOUT, 0); + _con_curl_check_connectivity (priv->concheck.curl_mhandle, CURL_SOCKET_TIMEOUT, 0); return G_SOURCE_REMOVE; } @@ -307,17 +317,34 @@ multi_timer_cb (CURLM *multi, long timeout_ms, void *userdata) nm_clear_g_source (&priv->concheck.curl_timer); if (timeout_ms != -1) - priv->concheck.curl_timer = g_timeout_add (timeout_ms, curl_timeout_cb, self); + priv->concheck.curl_timer = g_timeout_add (timeout_ms, _con_curl_timeout_cb, self); return 0; } +typedef struct { + NMConnectivity *self; + GIOChannel *ch; + + /* this is a very simplistic weak-pointer. If ConCurlSockData gets + * destroyed, it will set *destroy_notify to TRUE. + * + * _con_curl_socketevent_cb() uses this to detect whether it can + * safely access @fdp after _con_curl_check_connectivity(). */ + gboolean *destroy_notify; + + guint ev; +} ConCurlSockData; + static gboolean -curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_data) +_con_curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_data) { - gs_unref_object NMConnectivity *self = g_object_ref (NM_CONNECTIVITY (user_data)); + ConCurlSockData *fdp = user_data; + gs_unref_object NMConnectivity *self = g_object_ref (fdp->self); NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); int fd = g_io_channel_unix_get_fd (ch); int action = 0; + gboolean fdp_destroyed = FALSE; + gboolean success; if (condition & G_IO_IN) action |= CURL_CSELECT_IN; @@ -326,35 +353,49 @@ curl_socketevent_cb (GIOChannel *ch, GIOCondition condition, gpointer user_data) if (condition & G_IO_ERR) action |= CURL_CSELECT_ERR; - curl_check_connectivity (priv->concheck.curl_mhandle, fd, action); - return G_SOURCE_CONTINUE; + nm_assert (!fdp->destroy_notify); + fdp->destroy_notify = &fdp_destroyed; + + success = _con_curl_check_connectivity (priv->concheck.curl_mhandle, fd, action); + + if (fdp_destroyed) { + /* hups. fdp got invalidated during _con_curl_check_connectivity(). That's fine, + * just don't touch it. */ + } else { + nm_assert (fdp->destroy_notify == &fdp_destroyed); + fdp->destroy_notify = NULL; + if (!success) + fdp->ev = 0; + } + + return success ? G_SOURCE_CONTINUE : G_SOURCE_REMOVE; } -typedef struct { - GIOChannel *ch; - guint ev; -} CurlSockData; - static int -multi_socket_cb (CURL *e_handle, curl_socket_t s, int what, void *userdata, void *socketp) +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); - CurlSockData *fdp = socketp; + ConCurlSockData *fdp = socketp; GIOCondition condition = 0; + (void) _NM_ENSURE_TYPE (int, fd); + if (what == CURL_POLL_REMOVE) { if (fdp) { - curl_multi_assign (priv->concheck.curl_mhandle, s, NULL); + if (fdp->destroy_notify) + *fdp->destroy_notify = TRUE; + curl_multi_assign (priv->concheck.curl_mhandle, fd, NULL); nm_clear_g_source (&fdp->ev); g_io_channel_unref (fdp->ch); - g_slice_free (CurlSockData, fdp); + g_slice_free (ConCurlSockData, fdp); } } else { if (!fdp) { - fdp = g_slice_new0 (CurlSockData); - fdp->ch = g_io_channel_unix_new (s); - curl_multi_assign (priv->concheck.curl_mhandle, s, fdp); + fdp = g_slice_new0 (ConCurlSockData); + fdp->self = self; + fdp->ch = g_io_channel_unix_new (fd); + curl_multi_assign (priv->concheck.curl_mhandle, fd, fdp); } else nm_clear_g_source (&fdp->ev); @@ -366,7 +407,7 @@ multi_socket_cb (CURL *e_handle, curl_socket_t s, int what, void *userdata, void condition = G_IO_IN | G_IO_OUT; if (condition) - fdp->ev = g_io_add_watch (fdp->ch, condition, curl_socketevent_cb, self); + fdp->ev = g_io_add_watch (fdp->ch, condition, _con_curl_socketevent_cb, fdp); } return CURLM_OK;