Remove potential deadlocks in waitpipe with net_*
authorRémi Denis-Courmont <rem@videolan.org>
Mon, 21 Jan 2008 20:35:19 +0000 (20:35 +0000)
committerRémi Denis-Courmont <rem@videolan.org>
Mon, 21 Jan 2008 20:35:19 +0000 (20:35 +0000)
src/misc/objects.c
src/network/io.c
src/network/tcp.c

index 7719615..3961878 100644 (file)
@@ -555,26 +555,26 @@ int __vlc_object_waitpipe( vlc_object_t *obj )
 
         vlc_spin_lock (&internals->spin);
         signaled = internals->b_signaled;
-        if ((!signaled) && (internals->pipes[0] == -1))
+        if (internals->pipes[0] == -1)
         {
             internals->pipes[0] = pfd[0];
             internals->pipes[1] = pfd[1];
         }
+        else
+            race = VLC_TRUE;
     }
     vlc_spin_unlock (&internals->spin);
 
-    if (race || signaled)
-    {
+    if (race)
+    {   /* Race condition: two threads call pipe() - unlikely */
         close (pfd[0]);
         close (pfd[1]);
-
-        if (signaled)
-        {   /* vlc_object_signal() was already invoked! */
-            errno = EINTR;
-            return -1;
-        }
     }
 
+    if (signaled)
+        /* Race condition: lc_object_signal() already invoked! */
+        while (write (internals->pipes[1], &(char){ 0 }, 1) < 0);
+
     return internals->pipes[0];
 }
 
index d8e21d3..33119db 100644 (file)
@@ -258,7 +258,8 @@ int *net_Listen (vlc_object_t *p_this, const char *psz_host,
  *****************************************************************************
  * Reads from a network socket.
  * If waitall is true, then we repeat until we have read the right amount of
- * data; in that case, a short count means EOF has been reached.
+ * data; in that case, a short count means EOF has been reached or the VLC
+ * object has been signaled.
  *****************************************************************************/
 ssize_t
 __net_Read (vlc_object_t *restrict p_this, int fd, const v_socket_t *vs,
@@ -270,27 +271,17 @@ __net_Read (vlc_object_t *restrict p_this, int fd, const v_socket_t *vs,
         { .fd = -1, .events = POLLIN },
     };
 
-    vlc_object_lock (p_this);
-    ufd[1].fd = vlc_object_waitpipe (p_this);
-
     while (i_buflen > 0)
     {
         int val;
 
-        if (!vlc_object_alive (p_this))
+        ufd[1].fd = vlc_object_waitpipe (p_this);
+        if (ufd[1].fd == -1)
         {
-#if defined(WIN32) || defined(UNDER_CE)
-            WSASetLastError (WSAEINTR);
-#else
-            errno = EINTR;
-#endif
-            goto error;
         }
         ufd[0].revents = ufd[1].revents = 0;
 
-        vlc_object_unlock (p_this);
         val = poll (ufd, sizeof (ufd) / sizeof (ufd[0]), -1);
-        vlc_object_lock (p_this);
 
         if (val < 0)
             goto error;
@@ -298,9 +289,16 @@ __net_Read (vlc_object_t *restrict p_this, int fd, const v_socket_t *vs,
         if (ufd[1].revents)
         {
             msg_Dbg (p_this, "socket %d polling interrupted", fd);
-            vlc_object_wait (p_this);
-            msg_Dbg (p_this, "socket %d polling restarting", fd);
-            continue;
+            if (i_total == 0)
+            {
+#if defined(WIN32) || defined(UNDER_CE)
+                WSASetLastError (WSAEINTR);
+#else
+                errno = EINTR;
+#endif
+                goto error;
+            }
+            break;
         }
 
         assert (ufd[0].revents);
@@ -378,13 +376,10 @@ __net_Read (vlc_object_t *restrict p_this, int fd, const v_socket_t *vs,
             break;
     }
 
-    vlc_object_unlock (p_this);
     return i_total;
 
 error:
     msg_Err (p_this, "Read error: %m");
-
-    vlc_object_unlock (p_this);
     return -1;
 }
 
index 2f85155..0452170 100644 (file)
@@ -286,10 +286,9 @@ int __net_Accept( vlc_object_t *p_this, int *pi_fd, mtime_t i_wait )
 
     assert( pi_fd != NULL );
 
-    vlc_object_lock (p_this);
     evfd = vlc_object_waitpipe (p_this);
 
-    while (vlc_object_alive (p_this))
+    for (;;)
     {
         unsigned n = 0;
         while (pi_fd[n] != -1)
@@ -303,8 +302,9 @@ int __net_Accept( vlc_object_t *p_this, int *pi_fd, mtime_t i_wait )
             ufd[i].events = POLLIN;
             ufd[i].revents = 0;
         }
+        if (evfd == -1)
+            n--; /* avoid EBADF */
 
-        vlc_object_unlock (p_this);
         switch (poll (ufd, n, timeout))
         {
             case -1:
@@ -313,11 +313,9 @@ int __net_Accept( vlc_object_t *p_this, int *pi_fd, mtime_t i_wait )
             case 0:
                 return -1; /* NOTE: p_this already unlocked */
         }
-        vlc_object_lock (p_this);
 
         if (ufd[n].revents)
         {
-            vlc_object_wait (p_this);
             errno = EINTR;
             break;
         }
@@ -338,11 +336,9 @@ int __net_Accept( vlc_object_t *p_this, int *pi_fd, mtime_t i_wait )
              */
             memmove (pi_fd + i, pi_fd + i + 1, n - (i + 1));
             pi_fd[n - 1] = sfd;
-            vlc_object_unlock (p_this);
             return fd;
         }
     }
-    vlc_object_unlock (p_this);
     return -1;
 }