From f9d95de490847808bcdea2096840e30bb97bbfa1 Mon Sep 17 00:00:00 2001 From: Ignacio Casal Quinteiro Date: Thu, 11 May 2017 17:51:56 +0200 Subject: [PATCH] Revert "GSocket: Fix race conditions on Win32 if multiple threads are waiting on conditions for the same socket" This reverts commit 799f8dcd46fb40ea206d9f1b5468db62cc00a72e. This patch seems to break applications that use GTask specific operations with GSocket. We will need to investigate a bit more on this issue but for now we revert it and leave it for the next major release. --- gio/gsocket.c | 94 ++++++++++++++------------------------------------- 1 file changed, 26 insertions(+), 68 deletions(-) diff --git a/gio/gsocket.c b/gio/gsocket.c index eea502ced..ba90be54b 100644 --- a/gio/gsocket.c +++ b/gio/gsocket.c @@ -252,14 +252,11 @@ struct _GSocketPrivate guint connect_pending : 1; #ifdef G_OS_WIN32 WSAEVENT event; - gboolean waiting; - DWORD waiting_result; int current_events; int current_errors; int selected_events; GList *requested_conditions; /* list of requested GIOCondition * */ GMutex win32_source_lock; - GCond win32_source_cond; #endif struct { @@ -346,10 +343,8 @@ socket_strerror (int err) static void _win32_unset_event_mask (GSocket *socket, int mask) { - g_mutex_lock (&socket->priv->win32_source_lock); socket->priv->current_events &= ~mask; socket->priv->current_errors &= ~mask; - g_mutex_unlock (&socket->priv->win32_source_lock); } #else #define win32_unset_event_mask(_socket, _mask) @@ -847,7 +842,6 @@ g_socket_finalize (GObject *object) g_assert (socket->priv->requested_conditions == NULL); g_mutex_clear (&socket->priv->win32_source_lock); - g_cond_clear (&socket->priv->win32_source_cond); #endif for (i = 0; i < RECV_ADDR_CACHE_SIZE; i++) @@ -1075,7 +1069,6 @@ g_socket_init (GSocket *socket) #ifdef G_OS_WIN32 socket->priv->event = WSA_INVALID_EVENT; g_mutex_init (&socket->priv->win32_source_lock); - g_cond_init (&socket->priv->win32_source_cond); #endif } @@ -2459,8 +2452,6 @@ g_socket_accept (GSocket *socket, while (TRUE) { - win32_unset_event_mask (socket, FD_ACCEPT); - if ((ret = accept (socket->priv->fd, NULL, 0)) < 0) { int errsv = get_socket_errno (); @@ -2475,6 +2466,8 @@ g_socket_accept (GSocket *socket, errsv == EAGAIN) #endif { + win32_unset_event_mask (socket, FD_ACCEPT); + if (socket->priv->blocking) { if (!g_socket_condition_wait (socket, @@ -2491,6 +2484,8 @@ g_socket_accept (GSocket *socket, break; } + win32_unset_event_mask (socket, FD_ACCEPT); + #ifdef G_OS_WIN32 { /* The socket inherits the accepting sockets event mask and even object, @@ -2579,8 +2574,6 @@ g_socket_connect (GSocket *socket, while (1) { - win32_unset_event_mask (socket, FD_CONNECT); - if (connect (socket->priv->fd, (struct sockaddr *) &buffer, g_socket_address_get_native_size (address)) < 0) { @@ -2595,6 +2588,8 @@ g_socket_connect (GSocket *socket, if (errsv == WSAEWOULDBLOCK) #endif { + win32_unset_event_mask (socket, FD_CONNECT); + if (socket->priv->blocking) { if (g_socket_condition_wait (socket, G_IO_OUT, cancellable, error)) @@ -2620,6 +2615,8 @@ g_socket_connect (GSocket *socket, break; } + win32_unset_event_mask (socket, FD_CONNECT); + socket->priv->connected_read = TRUE; socket->priv->connected_write = TRUE; @@ -2799,8 +2796,6 @@ g_socket_receive_with_timeout (GSocket *socket, while (1) { - win32_unset_event_mask (socket, FD_READ); - if ((ret = recv (socket->priv->fd, buffer, size, 0)) < 0) { int errsv = get_socket_errno (); @@ -2815,6 +2810,8 @@ g_socket_receive_with_timeout (GSocket *socket, errsv == EAGAIN) #endif { + win32_unset_event_mask (socket, FD_READ); + if (timeout != 0) { if (!block_on_timeout (socket, G_IO_IN, timeout, start_time, @@ -2825,10 +2822,14 @@ g_socket_receive_with_timeout (GSocket *socket, } } + win32_unset_event_mask (socket, FD_READ); + socket_set_error_lazy (error, errsv, _("Error receiving data: %s")); return -1; } + win32_unset_event_mask (socket, FD_READ); + break; } @@ -2994,8 +2995,6 @@ g_socket_send_with_timeout (GSocket *socket, while (1) { - win32_unset_event_mask (socket, FD_WRITE); - if ((ret = send (socket->priv->fd, buffer, size, G_SOCKET_DEFAULT_SEND_FLAGS)) < 0) { int errsv = get_socket_errno (); @@ -3010,6 +3009,8 @@ g_socket_send_with_timeout (GSocket *socket, errsv == EAGAIN) #endif { + win32_unset_event_mask (socket, FD_WRITE); + if (timeout != 0) { if (!block_on_timeout (socket, G_IO_OUT, timeout, start_time, @@ -3424,7 +3425,7 @@ remove_condition_watch (GSocket *socket, } static GIOCondition -update_condition_unlocked (GSocket *socket) +update_condition (GSocket *socket) { WSANETWORKEVENTS events; GIOCondition condition; @@ -3491,16 +3492,6 @@ update_condition_unlocked (GSocket *socket) return condition; } - -static GIOCondition -update_condition (GSocket *socket) -{ - GIOCondition res; - g_mutex_lock (&socket->priv->win32_source_lock); - res = update_condition_unlocked (socket); - g_mutex_unlock (&socket->priv->win32_source_lock); - return res; -} #endif typedef struct { @@ -3896,44 +3887,11 @@ g_socket_condition_timed_wait (GSocket *socket, if (timeout == -1) timeout = WSA_INFINITE; - g_mutex_lock (&socket->priv->win32_source_lock); - current_condition = update_condition_unlocked (socket); + current_condition = update_condition (socket); while ((condition & current_condition) == 0) { - if (!socket->priv->waiting) - { - socket->priv->waiting = TRUE; - socket->priv->waiting_result = 0; - g_mutex_unlock (&socket->priv->win32_source_lock); - - res = WSAWaitForMultipleEvents (num_events, events, FALSE, timeout, FALSE); - - g_mutex_lock (&socket->priv->win32_source_lock); - socket->priv->waiting = FALSE; - socket->priv->waiting_result = res; - g_cond_broadcast (&socket->priv->win32_source_cond); - } - else - { - if (timeout != WSA_INFINITE) - { - if (!g_cond_wait_until (&socket->priv->win32_source_cond, &socket->priv->win32_source_lock, timeout)) - { - res = WSA_WAIT_TIMEOUT; - break; - } - else - { - res = socket->priv->waiting_result; - } - } - else - { - g_cond_wait (&socket->priv->win32_source_cond, &socket->priv->win32_source_lock); - res = socket->priv->waiting_result; - } - } - + res = WSAWaitForMultipleEvents (num_events, events, + FALSE, timeout, FALSE); if (res == WSA_WAIT_FAILED) { int errsv = get_socket_errno (); @@ -3954,7 +3912,7 @@ g_socket_condition_timed_wait (GSocket *socket, if (g_cancellable_set_error_if_cancelled (cancellable, error)) break; - current_condition = update_condition_unlocked (socket); + current_condition = update_condition (socket); if (timeout != WSA_INFINITE) { @@ -3963,7 +3921,6 @@ g_socket_condition_timed_wait (GSocket *socket, timeout = 0; } } - g_mutex_unlock (&socket->priv->win32_source_lock); remove_condition_watch (socket, &condition); if (num_events > 1) g_cancellable_release_fd (cancellable); @@ -4461,8 +4418,6 @@ g_socket_send_message_with_timeout (GSocket *socket, while (1) { - win32_unset_event_mask (socket, FD_WRITE); - if (address) result = WSASendTo (socket->priv->fd, bufs, num_vectors, @@ -4484,6 +4439,8 @@ g_socket_send_message_with_timeout (GSocket *socket, if (errsv == WSAEWOULDBLOCK) { + win32_unset_event_mask (socket, FD_WRITE); + if (timeout != 0) { if (!block_on_timeout (socket, G_IO_OUT, timeout, @@ -4931,8 +4888,6 @@ g_socket_receive_message_with_timeout (GSocket *socket, /* do it */ while (1) { - win32_unset_event_mask (socket, FD_READ); - addrlen = sizeof addr; if (address) result = WSARecvFrom (socket->priv->fd, @@ -4954,6 +4909,8 @@ g_socket_receive_message_with_timeout (GSocket *socket, if (errsv == WSAEWOULDBLOCK) { + win32_unset_event_mask (socket, FD_READ); + if (timeout != 0) { if (!block_on_timeout (socket, G_IO_IN, timeout, @@ -4967,6 +4924,7 @@ g_socket_receive_message_with_timeout (GSocket *socket, socket_set_error_lazy (error, errsv, _("Error receiving message: %s")); return -1; } + win32_unset_event_mask (socket, FD_READ); break; }