ecore: Add thread safety support to ecore pipe

Fixes hang in edje_codegen with thread safe ecore enabled.
ecore_shutdown takes lock, then calls _ecore_pipe_read, which
makes callbacks without unlocking ecore.

Signed-off-by: Mike McCormack <mikem@atratus.org>

SVN revision: 78937

4 files changed, 246 insertions, 159 deletions

diff --git a/legacy/ecore/src/lib/ecore/ecore.c b/legacy/ecore/src/lib/ecore/ecore.c
index 45a32d383..72c735ccd 100644
--- a/legacy/ecore/src/lib/ecore/ecore.c
+++ b/legacy/ecore/src/lib/ecore/ecore.c
@@ -181,7 +181,7 @@ ecore_init(void)
    eina_condition_new(&_thread_cond, &_thread_mutex);
    eina_lock_new(&_thread_feedback_mutex);
    eina_condition_new(&_thread_feedback_cond, &_thread_feedback_mutex);
-   _thread_call = ecore_pipe_add(_thread_callback, NULL);
+   _thread_call = _ecore_pipe_add(_thread_callback, NULL);
    eina_lock_new(&_thread_safety);
 
    eina_lock_new(&_thread_id_lock);
@@ -268,8 +268,8 @@ ecore_shutdown(void)
     */
      p = _thread_call;
      _thread_call = NULL;
-     ecore_pipe_wait(p, 1, 0.1);
-     ecore_pipe_del(p);
+     _ecore_pipe_wait(p, 1, 0.1);
+     _ecore_pipe_del(p);
      eina_lock_free(&_thread_safety);
      eina_condition_free(&_thread_cond);
      eina_lock_free(&_thread_mutex);
diff --git a/legacy/ecore/src/lib/ecore/ecore_main.c b/legacy/ecore/src/lib/ecore/ecore_main.c
index fea9fa85d..96deb4577 100644
--- a/legacy/ecore/src/lib/ecore/ecore_main.c
+++ b/legacy/ecore/src/lib/ecore/ecore_main.c
@@ -996,23 +996,20 @@ ecore_main_loop_select_func_get(void)
    return main_loop_select;
 }
 
-EAPI Ecore_Fd_Handler *
-ecore_main_fd_handler_add(int                    fd,
-                          Ecore_Fd_Handler_Flags flags,
-                          Ecore_Fd_Cb            func,
-                          const void            *data,
-                          Ecore_Fd_Cb            buf_func,
-                          const void            *buf_data)
+Ecore_Fd_Handler *
+_ecore_main_fd_handler_add(int                    fd,
+                           Ecore_Fd_Handler_Flags flags,
+                           Ecore_Fd_Cb            func,
+                           const void            *data,
+                           Ecore_Fd_Cb            buf_func,
+                           const void            *buf_data)
 {
    Ecore_Fd_Handler *fdh = NULL;
 
-   EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
-   _ecore_lock();
-
-   if ((fd < 0) || (flags == 0) || (!func)) goto unlock;
+   if ((fd < 0) || (flags == 0) || (!func)) return NULL;
 
    fdh = ecore_fd_handler_calloc(1);
-   if (!fdh) goto unlock;
+   if (!fdh) return NULL;
    ECORE_MAGIC_SET(fdh, ECORE_MAGIC_FD_HANDLER);
    fdh->next_ready = NULL;
    fdh->fd = fd;
@@ -1022,8 +1019,7 @@ ecore_main_fd_handler_add(int                    fd,
         int err = errno;
         ERR("Failed to add poll on fd %d (errno = %d: %s)!", fd, err, strerror(err));
         ecore_fd_handler_mp_free(fdh);
-        fdh = NULL;
-        goto unlock;
+        return NULL;
      }
    fdh->read_active = EINA_FALSE;
    fdh->write_active = EINA_FALSE;
@@ -1038,13 +1034,27 @@ ecore_main_fd_handler_add(int                    fd,
    fd_handlers = (Ecore_Fd_Handler *)
      eina_inlist_append(EINA_INLIST_GET(fd_handlers),
                         EINA_INLIST_GET(fdh));
-unlock:
-   _ecore_unlock();
 
    return fdh;
 }
 
 EAPI Ecore_Fd_Handler *
+ecore_main_fd_handler_add(int                    fd,
+                          Ecore_Fd_Handler_Flags flags,
+                          Ecore_Fd_Cb            func,
+                          const void            *data,
+                          Ecore_Fd_Cb            buf_func,
+                          const void            *buf_data)
+{
+   Ecore_Fd_Handler *fdh = NULL;
+   EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
+   _ecore_lock();
+   fdh = _ecore_main_fd_handler_add(fd, flags, func, data, buf_func, buf_data);
+   _ecore_unlock();
+   return fdh;
+}
+
+EAPI Ecore_Fd_Handler *
 ecore_main_fd_handler_file_add(int                    fd,
                                Ecore_Fd_Handler_Flags flags,
                                Ecore_Fd_Cb            func,
diff --git a/legacy/ecore/src/lib/ecore/ecore_pipe.c b/legacy/ecore/src/lib/ecore/ecore_pipe.c
index debdbe846..54477dfb4 100644
--- a/legacy/ecore/src/lib/ecore/ecore_pipe.c
+++ b/legacy/ecore/src/lib/ecore/ecore_pipe.c
@@ -123,32 +123,11 @@ ecore_pipe_add(Ecore_Pipe_Cb handler,
                const void   *data)
 {
    Ecore_Pipe *p;
-   int fds[2];
-
-   EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
-   if (!handler) return NULL;
 
-   p = ecore_pipe_calloc(1);
-   if (!p) return NULL;
-
-   if (pipe(fds))
-     {
-        ecore_pipe_mp_free(p);
-        return NULL;
-     }
+   _ecore_lock();
+   p = _ecore_pipe_add(handler, data);
+   _ecore_unlock();
 
-   ECORE_MAGIC_SET(p, ECORE_MAGIC_PIPE);
-   p->fd_read = fds[0];
-   p->fd_write = fds[1];
-   p->handler = handler;
-   p->data = data;
-
-   fcntl(p->fd_read, F_SETFL, O_NONBLOCK);
-   p->fd_handler = ecore_main_fd_handler_add(p->fd_read,
-                                             ECORE_FD_READ,
-                                             _ecore_pipe_read,
-                                             p,
-                                             NULL, NULL);
    return p;
 }
 
@@ -161,22 +140,12 @@ ecore_pipe_add(Ecore_Pipe_Cb handler,
 EAPI void *
 ecore_pipe_del(Ecore_Pipe *p)
 {
-   void *data;
-
+   void *r;
    EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
-   if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
-     {
-        ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_del");
-        return NULL;
-     }
-   p->delete_me = EINA_TRUE;
-   if (p->handling > 0) return (void *)p->data;
-   if (p->fd_handler) _ecore_main_fd_handler_del(p->fd_handler);
-   if (p->fd_read != PIPE_FD_INVALID) pipe_close(p->fd_read);
-   if (p->fd_write != PIPE_FD_INVALID) pipe_close(p->fd_write);
-   data = (void *)p->data;
-   ecore_pipe_mp_free(p);
-   return data;
+   _ecore_lock();
+   r = _ecore_pipe_del(p);
+   _ecore_unlock();
+   return r;
 }
 
 /**
@@ -188,10 +157,11 @@ EAPI void
 ecore_pipe_read_close(Ecore_Pipe *p)
 {
    EINA_MAIN_LOOP_CHECK_RETURN;
+   _ecore_lock();
    if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
      {
         ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_read_close");
-        return;
+        goto out;
      }
    if (p->fd_handler)
      {
@@ -203,6 +173,8 @@ ecore_pipe_read_close(Ecore_Pipe *p)
         pipe_close(p->fd_read);
         p->fd_read = PIPE_FD_INVALID;
      }
+out:
+   _ecore_unlock();
 }
 
 /**
@@ -216,16 +188,19 @@ EAPI void
 ecore_pipe_freeze(Ecore_Pipe *p)
 {
    EINA_MAIN_LOOP_CHECK_RETURN;
+   _ecore_lock();
    if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
      {
         ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_read_freeze");
-        return;
+        goto out;
      }
    if (p->fd_handler)
      {
         _ecore_main_fd_handler_del(p->fd_handler);
         p->fd_handler = NULL;
      }
+out:
+   _ecore_unlock();
 }
 
 /**
@@ -240,10 +215,11 @@ EAPI void
 ecore_pipe_thaw(Ecore_Pipe *p)
 {
    EINA_MAIN_LOOP_CHECK_RETURN;
+   _ecore_lock();
    if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
      {
         ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_read_thaw");
-        return;
+        goto out;
      }
    if (!p->fd_handler && p->fd_read != PIPE_FD_INVALID)
      {
@@ -253,6 +229,8 @@ ecore_pipe_thaw(Ecore_Pipe *p)
                                                   p,
                                                   NULL, NULL);
      }
+out:
+   _ecore_unlock();
 }
 
 /**
@@ -271,80 +249,11 @@ ecore_pipe_wait(Ecore_Pipe *p,
                 int         message_count,
                 double      wait)
 {
-   struct timeval tv, *t;
-   fd_set rset;
-   double end = 0.0;
-   double timeout;
-   int ret;
-   int total = 0;
-
-   EINA_MAIN_LOOP_CHECK_RETURN_VAL(-1);
-   if (p->fd_read == PIPE_FD_INVALID)
-     return -1;
-
-   FD_ZERO(&rset);
-   FD_SET(p->fd_read, &rset);
-
-   if (wait >= 0.0)
-     end = ecore_loop_time_get() + wait;
-   timeout = wait;
-
-   while (message_count > 0 && (timeout > 0.0 || wait <= 0.0))
-     {
-        if (wait >= 0.0)
-          {
-             /* finite() tests for NaN, too big, too small, and infinity.  */
-              if ((!ECORE_FINITE(timeout)) || (timeout == 0.0))
-                {
-                   tv.tv_sec = 0;
-                   tv.tv_usec = 0;
-                }
-              else if (timeout > 0.0)
-                {
-                   int sec, usec;
-#ifdef FIX_HZ
-                   timeout += (0.5 / HZ);
-                   sec = (int)timeout;
-                   usec = (int)((timeout - (double)sec) * 1000000);
-#else
-                   sec = (int)timeout;
-                   usec = (int)((timeout - (double)sec) * 1000000);
-#endif
-                   tv.tv_sec = sec;
-                   tv.tv_usec = usec;
-                }
-              t = &tv;
-          }
-        else
-          {
-             t = NULL;
-          }
-
-        ret = main_loop_select(p->fd_read + 1, &rset, NULL, NULL, t);
-
-        if (ret > 0)
-          {
-             _ecore_pipe_read(p, NULL);
-             message_count -= p->message;
-             total += p->message;
-             p->message = 0;
-          }
-        else if (ret == 0)
-          {
-             break;
-          }
-        else if (errno != EINTR)
-          {
-             close(p->fd_read);
-             p->fd_read = PIPE_FD_INVALID;
-             break;
-          }
-
-        if (wait >= 0.0)
-          timeout = end - ecore_loop_time_get();
-     }
-
-   return total;
+   int r;
+   _ecore_lock();
+   r = _ecore_pipe_wait(p, message_count, wait);
+   _ecore_unlock();
+   return r;
 }
 
 /**
@@ -355,16 +264,19 @@ ecore_pipe_wait(Ecore_Pipe *p,
 EAPI void
 ecore_pipe_write_close(Ecore_Pipe *p)
 {
+   _ecore_lock();
    if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
      {
         ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_write_close");
-        return;
+        goto out;
      }
    if (p->fd_write != PIPE_FD_INVALID)
      {
         pipe_close(p->fd_write);
         p->fd_write = PIPE_FD_INVALID;
      }
+out:
+   _ecore_unlock();
 }
 
 /**
@@ -383,16 +295,18 @@ ecore_pipe_write(Ecore_Pipe  *p,
    ssize_t ret;
    size_t already_written = 0;
    int retry = ECORE_PIPE_WRITE_RETRY;
+   Eina_Bool ok = EINA_FALSE;
 
+   _ecore_lock();
    if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
      {
         ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_write");
-        return EINA_FALSE;
+        goto out;
      }
 
-   if (p->delete_me) return EINA_FALSE;
+   if (p->delete_me) goto out;
 
-   if (p->fd_write == PIPE_FD_INVALID) return EINA_FALSE;
+   if (p->fd_write == PIPE_FD_INVALID) goto out;
 
    /* First write the len into the pipe */
    do
@@ -408,13 +322,13 @@ ecore_pipe_write(Ecore_Pipe  *p,
              /* XXX What should we do here? */
               ERR("The length of the data was not written complete"
                   " to the pipe");
-              return EINA_FALSE;
+              goto out;
           }
         else if (ret == PIPE_FD_ERROR && errno == EPIPE)
           {
              pipe_close(p->fd_write);
              p->fd_write = PIPE_FD_INVALID;
-             return EINA_FALSE;
+             goto out;
           }
         else if (ret == PIPE_FD_ERROR && errno == EINTR)
           /* try it again */
@@ -428,7 +342,7 @@ ecore_pipe_write(Ecore_Pipe  *p,
      }
    while (retry--);
 
-   if (retry != ECORE_PIPE_WRITE_RETRY) return EINA_FALSE;
+   if (retry != ECORE_PIPE_WRITE_RETRY) goto out;
 
    /* and now pass the data to the pipe */
    do
@@ -438,7 +352,10 @@ ecore_pipe_write(Ecore_Pipe  *p,
                          nbytes - already_written);
 
         if (ret == (ssize_t)(nbytes - already_written))
-          return EINA_TRUE;
+          {
+            ok = EINA_TRUE;
+            goto out;
+          }
         else if (ret >= 0)
           {
              already_written -= ret;
@@ -448,7 +365,7 @@ ecore_pipe_write(Ecore_Pipe  *p,
           {
              pipe_close(p->fd_write);
              p->fd_write = PIPE_FD_INVALID;
-             return EINA_FALSE;
+             goto out;
           }
         else if (ret == PIPE_FD_ERROR && errno == EINTR)
           /* try it again */
@@ -462,21 +379,173 @@ ecore_pipe_write(Ecore_Pipe  *p,
      }
    while (retry--);
 
-   return EINA_FALSE;
+out:
+   _ecore_unlock();
+   return ok;
 }
 
 /**
  * @}
  */
 
-/* Private function */
+/* Private functions */
+Ecore_Pipe *
+_ecore_pipe_add(Ecore_Pipe_Cb handler,
+                const void   *data)
+{
+   Ecore_Pipe *p = NULL;
+   int fds[2];
+
+   EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
+   if (!handler) return NULL;
+
+   p = ecore_pipe_calloc(1);
+   if (!p) return NULL;
+
+   if (pipe(fds))
+     {
+        ecore_pipe_mp_free(p);
+        return NULL;
+     }
+
+   ECORE_MAGIC_SET(p, ECORE_MAGIC_PIPE);
+   p->fd_read = fds[0];
+   p->fd_write = fds[1];
+   p->handler = handler;
+   p->data = data;
+
+   fcntl(p->fd_read, F_SETFL, O_NONBLOCK);
+   p->fd_handler = ecore_main_fd_handler_add(p->fd_read,
+                                             ECORE_FD_READ,
+                                             _ecore_pipe_read,
+                                             p,
+                                             NULL, NULL);
+
+   return p;
+}
+
+void *
+_ecore_pipe_del(Ecore_Pipe *p)
+{
+   void *data = NULL;
+
+   if (!ECORE_MAGIC_CHECK(p, ECORE_MAGIC_PIPE))
+     {
+        ECORE_MAGIC_FAIL(p, ECORE_MAGIC_PIPE, "ecore_pipe_del");
+        return NULL;
+     }
+   p->delete_me = EINA_TRUE;
+   if (p->handling > 0) return (void *)p->data;
+   if (p->fd_handler) _ecore_main_fd_handler_del(p->fd_handler);
+   if (p->fd_read != PIPE_FD_INVALID) pipe_close(p->fd_read);
+   if (p->fd_write != PIPE_FD_INVALID) pipe_close(p->fd_write);
+   data = (void *)p->data;
+   ecore_pipe_mp_free(p);
+   return data;
+}
+
+int
+_ecore_pipe_wait(Ecore_Pipe *p,
+                 int         message_count,
+                 double      wait)
+{
+   struct timeval tv, *t;
+   fd_set rset;
+   double end = 0.0;
+   double timeout;
+   int ret;
+   int total = 0;
+
+   EINA_MAIN_LOOP_CHECK_RETURN_VAL(-1);
+   if (p->fd_read == PIPE_FD_INVALID)
+     return -1;
+
+   FD_ZERO(&rset);
+   FD_SET(p->fd_read, &rset);
+
+   if (wait >= 0.0)
+     end = ecore_loop_time_get() + wait;
+   timeout = wait;
+
+   while (message_count > 0 && (timeout > 0.0 || wait <= 0.0))
+     {
+        if (wait >= 0.0)
+          {
+             /* finite() tests for NaN, too big, too small, and infinity.  */
+              if ((!ECORE_FINITE(timeout)) || (timeout == 0.0))
+                {
+                   tv.tv_sec = 0;
+                   tv.tv_usec = 0;
+                }
+              else if (timeout > 0.0)
+                {
+                   int sec, usec;
+#ifdef FIX_HZ
+                   timeout += (0.5 / HZ);
+                   sec = (int)timeout;
+                   usec = (int)((timeout - (double)sec) * 1000000);
+#else
+                   sec = (int)timeout;
+                   usec = (int)((timeout - (double)sec) * 1000000);
+#endif
+                   tv.tv_sec = sec;
+                   tv.tv_usec = usec;
+                }
+              t = &tv;
+          }
+        else
+          {
+             t = NULL;
+          }
+
+        ret = main_loop_select(p->fd_read + 1, &rset, NULL, NULL, t);
+
+        if (ret > 0)
+          {
+             _ecore_pipe_read(p, NULL);
+             message_count -= p->message;
+             total += p->message;
+             p->message = 0;
+          }
+        else if (ret == 0)
+          {
+             break;
+          }
+        else if (errno != EINTR)
+          {
+             close(p->fd_read);
+             p->fd_read = PIPE_FD_INVALID;
+             break;
+          }
+
+        if (wait >= 0.0)
+          timeout = end - ecore_loop_time_get();
+     }
+
+   return total;
+}
+
 static void
 _ecore_pipe_unhandle(Ecore_Pipe *p)
 {
    p->handling--;
    if (p->delete_me)
      {
-        ecore_pipe_del(p);
+        _ecore_pipe_del(p);
+     }
+}
+
+static void
+_ecore_pipe_handler_call(Ecore_Pipe *p,
+                         unsigned char *buf,
+                         size_t len)
+{
+   void *data = (void*) p->data;
+   if (!p->delete_me)
+     {
+        _ecore_unlock();
+        p->handler(data, buf, len);
+        _ecore_lock();
      }
 }
 
@@ -519,8 +588,7 @@ _ecore_pipe_read(void             *data,
                     if (i == 0)
                       {
      /* no data on first try through means an error */
-                          if (!p->delete_me)
-                            p->handler((void *)p->data, NULL, 0);
+                          _ecore_pipe_handler_call(p, NULL, 0);
                           if (p->passed_data) free(p->passed_data);
                           p->passed_data = NULL;
                           p->already_read = 0;
@@ -557,8 +625,7 @@ _ecore_pipe_read(void             *data,
                 {
                    if (WSAGetLastError() != WSAEWOULDBLOCK)
                      {
-                        if (!p->delete_me)
-                          p->handler((void *)p->data, NULL, 0);
+                        _ecore_pipe_handler_call(p, NULL, 0);
                         if (p->passed_data) free(p->passed_data);
                         p->passed_data = NULL;
                         p->already_read = 0;
@@ -579,8 +646,7 @@ _ecore_pipe_read(void             *data,
          * never happen */
         if (p->len == 0)
           {
-             if (!p->delete_me)
-               p->handler((void *)p->data, NULL, 0);
+             _ecore_pipe_handler_call(p, NULL, 0);
              /* reset all values to 0 */
              if (p->passed_data) free(p->passed_data);
              p->passed_data = NULL;
@@ -598,8 +664,7 @@ _ecore_pipe_read(void             *data,
              /* alloc failed - error case */
              if (!p->passed_data)
                {
-                  if (!p->delete_me)
-                    p->handler((void *)p->data, NULL, 0);
+                  _ecore_pipe_handler_call(p, NULL, 0);
      /* close the pipe */
                   p->already_read = 0;
                   p->len = 0;
@@ -621,8 +686,7 @@ _ecore_pipe_read(void             *data,
         /* if we read enough data to finish the message/buffer */
         if (ret == (ssize_t)(p->len - p->already_read))
           {
-             if (!p->delete_me)
-               p->handler((void *)p->data, p->passed_data, p->len);
+             _ecore_pipe_handler_call(p, p->passed_data, p->len);
              free(p->passed_data);
              /* reset all values to 0 */
              p->passed_data = NULL;
@@ -663,8 +727,7 @@ _ecore_pipe_read(void             *data,
           {
              if (WSAGetLastError() != WSAEWOULDBLOCK)
                {
-                  if (!p->delete_me)
-                    p->handler((void *)p->data, NULL, 0);
+                  _ecore_pipe_handler_call(p, NULL, 0);
                   if (p->passed_data) free(p->passed_data);
                   p->passed_data = NULL;
                   p->already_read = 0;
diff --git a/legacy/ecore/src/lib/ecore/ecore_private.h b/legacy/ecore/src/lib/ecore/ecore_private.h
index ce1199765..f0add822f 100644
--- a/legacy/ecore/src/lib/ecore/ecore_private.h
+++ b/legacy/ecore/src/lib/ecore/ecore_private.h
@@ -170,6 +170,20 @@ void      *_ecore_event_signal_exit_new(void);
 void      *_ecore_event_signal_power_new(void);
 void      *_ecore_event_signal_realtime_new(void);
 
+Ecore_Pipe *_ecore_pipe_add(Ecore_Pipe_Cb handler,
+                            const void   *data);
+int         _ecore_pipe_wait(Ecore_Pipe *p,
+                             int message_count,
+                             double wait);
+void       *_ecore_pipe_del(Ecore_Pipe *p);
+
+Ecore_Fd_Handler *
+           _ecore_main_fd_handler_add(int fd,
+                                      Ecore_Fd_Handler_Flags flags,
+                                      Ecore_Fd_Cb func,
+                                      const void *data,
+                                      Ecore_Fd_Cb buf_func,
+                                      const void *buf_data);
 void      *_ecore_main_fd_handler_del(Ecore_Fd_Handler *fd_handler);
 
 void       _ecore_main_shutdown(void);