Revert "GSocket: Fix race conditions on Win32 if multiple threads are waiting on conditions for the same socket"

This reverts commit 799f8dcd46.
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.
This commit is contained in:
Ignacio Casal Quinteiro 2017-05-11 17:51:56 +02:00 committed by Ignacio Casal Quinteiro
parent 51e852e5d0
commit f9d95de490

View File

@ -252,14 +252,11 @@ struct _GSocketPrivate
guint connect_pending : 1; guint connect_pending : 1;
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
WSAEVENT event; WSAEVENT event;
gboolean waiting;
DWORD waiting_result;
int current_events; int current_events;
int current_errors; int current_errors;
int selected_events; int selected_events;
GList *requested_conditions; /* list of requested GIOCondition * */ GList *requested_conditions; /* list of requested GIOCondition * */
GMutex win32_source_lock; GMutex win32_source_lock;
GCond win32_source_cond;
#endif #endif
struct { struct {
@ -346,10 +343,8 @@ socket_strerror (int err)
static void static void
_win32_unset_event_mask (GSocket *socket, int mask) _win32_unset_event_mask (GSocket *socket, int mask)
{ {
g_mutex_lock (&socket->priv->win32_source_lock);
socket->priv->current_events &= ~mask; socket->priv->current_events &= ~mask;
socket->priv->current_errors &= ~mask; socket->priv->current_errors &= ~mask;
g_mutex_unlock (&socket->priv->win32_source_lock);
} }
#else #else
#define win32_unset_event_mask(_socket, _mask) #define win32_unset_event_mask(_socket, _mask)
@ -847,7 +842,6 @@ g_socket_finalize (GObject *object)
g_assert (socket->priv->requested_conditions == NULL); g_assert (socket->priv->requested_conditions == NULL);
g_mutex_clear (&socket->priv->win32_source_lock); g_mutex_clear (&socket->priv->win32_source_lock);
g_cond_clear (&socket->priv->win32_source_cond);
#endif #endif
for (i = 0; i < RECV_ADDR_CACHE_SIZE; i++) for (i = 0; i < RECV_ADDR_CACHE_SIZE; i++)
@ -1075,7 +1069,6 @@ g_socket_init (GSocket *socket)
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
socket->priv->event = WSA_INVALID_EVENT; socket->priv->event = WSA_INVALID_EVENT;
g_mutex_init (&socket->priv->win32_source_lock); g_mutex_init (&socket->priv->win32_source_lock);
g_cond_init (&socket->priv->win32_source_cond);
#endif #endif
} }
@ -2459,8 +2452,6 @@ g_socket_accept (GSocket *socket,
while (TRUE) while (TRUE)
{ {
win32_unset_event_mask (socket, FD_ACCEPT);
if ((ret = accept (socket->priv->fd, NULL, 0)) < 0) if ((ret = accept (socket->priv->fd, NULL, 0)) < 0)
{ {
int errsv = get_socket_errno (); int errsv = get_socket_errno ();
@ -2475,6 +2466,8 @@ g_socket_accept (GSocket *socket,
errsv == EAGAIN) errsv == EAGAIN)
#endif #endif
{ {
win32_unset_event_mask (socket, FD_ACCEPT);
if (socket->priv->blocking) if (socket->priv->blocking)
{ {
if (!g_socket_condition_wait (socket, if (!g_socket_condition_wait (socket,
@ -2491,6 +2484,8 @@ g_socket_accept (GSocket *socket,
break; break;
} }
win32_unset_event_mask (socket, FD_ACCEPT);
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
{ {
/* The socket inherits the accepting sockets event mask and even object, /* The socket inherits the accepting sockets event mask and even object,
@ -2579,8 +2574,6 @@ g_socket_connect (GSocket *socket,
while (1) while (1)
{ {
win32_unset_event_mask (socket, FD_CONNECT);
if (connect (socket->priv->fd, (struct sockaddr *) &buffer, if (connect (socket->priv->fd, (struct sockaddr *) &buffer,
g_socket_address_get_native_size (address)) < 0) g_socket_address_get_native_size (address)) < 0)
{ {
@ -2595,6 +2588,8 @@ g_socket_connect (GSocket *socket,
if (errsv == WSAEWOULDBLOCK) if (errsv == WSAEWOULDBLOCK)
#endif #endif
{ {
win32_unset_event_mask (socket, FD_CONNECT);
if (socket->priv->blocking) if (socket->priv->blocking)
{ {
if (g_socket_condition_wait (socket, G_IO_OUT, cancellable, error)) if (g_socket_condition_wait (socket, G_IO_OUT, cancellable, error))
@ -2620,6 +2615,8 @@ g_socket_connect (GSocket *socket,
break; break;
} }
win32_unset_event_mask (socket, FD_CONNECT);
socket->priv->connected_read = TRUE; socket->priv->connected_read = TRUE;
socket->priv->connected_write = TRUE; socket->priv->connected_write = TRUE;
@ -2799,8 +2796,6 @@ g_socket_receive_with_timeout (GSocket *socket,
while (1) while (1)
{ {
win32_unset_event_mask (socket, FD_READ);
if ((ret = recv (socket->priv->fd, buffer, size, 0)) < 0) if ((ret = recv (socket->priv->fd, buffer, size, 0)) < 0)
{ {
int errsv = get_socket_errno (); int errsv = get_socket_errno ();
@ -2815,6 +2810,8 @@ g_socket_receive_with_timeout (GSocket *socket,
errsv == EAGAIN) errsv == EAGAIN)
#endif #endif
{ {
win32_unset_event_mask (socket, FD_READ);
if (timeout != 0) if (timeout != 0)
{ {
if (!block_on_timeout (socket, G_IO_IN, timeout, start_time, 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")); socket_set_error_lazy (error, errsv, _("Error receiving data: %s"));
return -1; return -1;
} }
win32_unset_event_mask (socket, FD_READ);
break; break;
} }
@ -2994,8 +2995,6 @@ g_socket_send_with_timeout (GSocket *socket,
while (1) while (1)
{ {
win32_unset_event_mask (socket, FD_WRITE);
if ((ret = send (socket->priv->fd, buffer, size, G_SOCKET_DEFAULT_SEND_FLAGS)) < 0) if ((ret = send (socket->priv->fd, buffer, size, G_SOCKET_DEFAULT_SEND_FLAGS)) < 0)
{ {
int errsv = get_socket_errno (); int errsv = get_socket_errno ();
@ -3010,6 +3009,8 @@ g_socket_send_with_timeout (GSocket *socket,
errsv == EAGAIN) errsv == EAGAIN)
#endif #endif
{ {
win32_unset_event_mask (socket, FD_WRITE);
if (timeout != 0) if (timeout != 0)
{ {
if (!block_on_timeout (socket, G_IO_OUT, timeout, start_time, if (!block_on_timeout (socket, G_IO_OUT, timeout, start_time,
@ -3424,7 +3425,7 @@ remove_condition_watch (GSocket *socket,
} }
static GIOCondition static GIOCondition
update_condition_unlocked (GSocket *socket) update_condition (GSocket *socket)
{ {
WSANETWORKEVENTS events; WSANETWORKEVENTS events;
GIOCondition condition; GIOCondition condition;
@ -3491,16 +3492,6 @@ update_condition_unlocked (GSocket *socket)
return condition; 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 #endif
typedef struct { typedef struct {
@ -3896,44 +3887,11 @@ g_socket_condition_timed_wait (GSocket *socket,
if (timeout == -1) if (timeout == -1)
timeout = WSA_INFINITE; timeout = WSA_INFINITE;
g_mutex_lock (&socket->priv->win32_source_lock); current_condition = update_condition (socket);
current_condition = update_condition_unlocked (socket);
while ((condition & current_condition) == 0) while ((condition & current_condition) == 0)
{ {
if (!socket->priv->waiting) res = WSAWaitForMultipleEvents (num_events, events,
{ FALSE, timeout, FALSE);
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;
}
}
if (res == WSA_WAIT_FAILED) if (res == WSA_WAIT_FAILED)
{ {
int errsv = get_socket_errno (); 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)) if (g_cancellable_set_error_if_cancelled (cancellable, error))
break; break;
current_condition = update_condition_unlocked (socket); current_condition = update_condition (socket);
if (timeout != WSA_INFINITE) if (timeout != WSA_INFINITE)
{ {
@ -3963,7 +3921,6 @@ g_socket_condition_timed_wait (GSocket *socket,
timeout = 0; timeout = 0;
} }
} }
g_mutex_unlock (&socket->priv->win32_source_lock);
remove_condition_watch (socket, &condition); remove_condition_watch (socket, &condition);
if (num_events > 1) if (num_events > 1)
g_cancellable_release_fd (cancellable); g_cancellable_release_fd (cancellable);
@ -4461,8 +4418,6 @@ g_socket_send_message_with_timeout (GSocket *socket,
while (1) while (1)
{ {
win32_unset_event_mask (socket, FD_WRITE);
if (address) if (address)
result = WSASendTo (socket->priv->fd, result = WSASendTo (socket->priv->fd,
bufs, num_vectors, bufs, num_vectors,
@ -4484,6 +4439,8 @@ g_socket_send_message_with_timeout (GSocket *socket,
if (errsv == WSAEWOULDBLOCK) if (errsv == WSAEWOULDBLOCK)
{ {
win32_unset_event_mask (socket, FD_WRITE);
if (timeout != 0) if (timeout != 0)
{ {
if (!block_on_timeout (socket, G_IO_OUT, timeout, if (!block_on_timeout (socket, G_IO_OUT, timeout,
@ -4931,8 +4888,6 @@ g_socket_receive_message_with_timeout (GSocket *socket,
/* do it */ /* do it */
while (1) while (1)
{ {
win32_unset_event_mask (socket, FD_READ);
addrlen = sizeof addr; addrlen = sizeof addr;
if (address) if (address)
result = WSARecvFrom (socket->priv->fd, result = WSARecvFrom (socket->priv->fd,
@ -4954,6 +4909,8 @@ g_socket_receive_message_with_timeout (GSocket *socket,
if (errsv == WSAEWOULDBLOCK) if (errsv == WSAEWOULDBLOCK)
{ {
win32_unset_event_mask (socket, FD_READ);
if (timeout != 0) if (timeout != 0)
{ {
if (!block_on_timeout (socket, G_IO_IN, timeout, 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")); socket_set_error_lazy (error, errsv, _("Error receiving message: %s"));
return -1; return -1;
} }
win32_unset_event_mask (socket, FD_READ);
break; break;
} }