[wing/wip/pipe-refactor: 1/2] namedpipelistener: rework how pipes are created



commit 5aa9944e3b44cf0856fa7be449a02d0d8043097e
Author: Ignacio Casal Quinteiro <icq gnome org>
Date:   Thu Jul 14 12:29:40 2016 +0200

    namedpipelistener: rework how pipes are created

 tests/named-pipe.c           |   43 +++--
 wing/wingnamedpipelistener.c |  439 +++++++++++++++++++++++++----------------
 2 files changed, 295 insertions(+), 187 deletions(-)
---
diff --git a/tests/named-pipe.c b/tests/named-pipe.c
index d9ac26c..2175b38 100644
--- a/tests/named-pipe.c
+++ b/tests/named-pipe.c
@@ -33,12 +33,6 @@ test_add_named_pipe (void)
                                            &error);
   g_assert_no_error (error);
 
-  wing_named_pipe_listener_add_named_pipe (listener,
-                                           "\\\\.\\gtest-bad-named-pipe-name",
-                                           NULL,
-                                           &error);
-  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
-
   g_object_unref (listener);
 }
 
@@ -114,12 +108,28 @@ test_connect_basic (void)
 }
 
 static void
+connect_failed_cb (GObject      *source,
+                   GAsyncResult *result,
+                   gpointer      user_data)
+{
+  WingNamedPipeClient *client = WING_NAMED_PIPE_CLIENT (source);
+  WingNamedPipeConnection *conn;
+  gboolean *failed = user_data;
+  GError *error = NULL;
+
+  conn = wing_named_pipe_client_connect_finish (client, result, &error);
+  g_assert_null (conn);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+
+  *failed = TRUE;
+}
+
+static void
 test_connect_before_accept (void)
 {
   WingNamedPipeListener *listener;
   WingNamedPipeClient *client;
-  gboolean success_accepted = FALSE;
-  gboolean success_connected = FALSE;
+  gboolean connect_failed = FALSE;
   GError *error = NULL;
 
   listener = wing_named_pipe_listener_new ();
@@ -134,17 +144,12 @@ test_connect_before_accept (void)
   wing_named_pipe_client_connect_async (client,
                                         "\\\\.\\pipe\\gtest-named-pipe-name",
                                         NULL,
-                                        connected_cb,
-                                        &success_connected);
-
-  wing_named_pipe_listener_accept_async (listener,
-                                         NULL,
-                                         accepted_cb,
-                                         &success_accepted);
+                                        connect_failed_cb,
+                                        &connect_failed);
 
   do
     g_main_context_iteration (NULL, TRUE);
-  while (!success_accepted || !success_connected);
+  while (!connect_failed);
 
   g_object_unref (client);
   g_object_unref (listener);
@@ -166,6 +171,12 @@ test_connect_sync (void)
                                            &error);
   g_assert_no_error (error);
 
+  /* Accept will do the connect server side so the client connect will not fail */
+  wing_named_pipe_listener_accept_async (listener,
+                                         NULL,
+                                         NULL,
+                                         NULL);
+
   client = wing_named_pipe_client_new ();
   connection = wing_named_pipe_client_connect (client,
                                                "\\\\.\\pipe\\gtest-connect-sync",
diff --git a/wing/wingnamedpipelistener.c b/wing/wingnamedpipelistener.c
index de2409a..0f69328 100644
--- a/wing/wingnamedpipelistener.c
+++ b/wing/wingnamedpipelistener.c
@@ -28,11 +28,17 @@
 typedef struct
 {
   gchar *pipe_name;
+  GObject *source_object;
+} PipeData;
+
+typedef struct
+{
+  PipeData *pdata;
   HANDLE handle;
   OVERLAPPED overlapped;
-  GObject *source_object;
   gboolean already_connected;
-} PipeData;
+  GError *error;
+} PipeHandleData;
 
 typedef struct
 {
@@ -46,18 +52,12 @@ static GQuark source_quark = 0;
 
 static PipeData *
 pipe_data_new (const gchar *pipe_name,
-               HANDLE       handle,
                GObject     *source_object)
 {
   PipeData *data;
 
   data = g_slice_new0 (PipeData);
   data->pipe_name = g_strdup (pipe_name);
-  data->handle = handle;
-  data->overlapped.hEvent = CreateEvent (NULL, /* default security attribute */
-                                         TRUE, /* manual-reset event */
-                                         TRUE, /* initial state = signaled */
-                                         NULL); /* unnamed event object */
   if (source_object)
     data->source_object = g_object_ref (source_object);
 
@@ -68,12 +68,37 @@ static void
 pipe_data_free (PipeData *data)
 {
   g_free (data->pipe_name);
-  CloseHandle (data->handle);
-  CloseHandle (data->overlapped.hEvent);
   g_clear_object (&data->source_object);
   g_slice_free (PipeData, data);
 }
 
+static PipeHandleData *
+pipe_handle_data_new (PipeData *data)
+{
+  PipeHandleData *handle_data;
+
+  handle_data = g_slice_new0 (PipeHandleData);
+  handle_data->pdata = data;
+  handle_data->handle = INVALID_HANDLE_VALUE;
+  handle_data->overlapped.hEvent = CreateEvent (NULL, /* default security attribute */
+                                                TRUE, /* manual-reset event */
+                                                TRUE, /* initial state = signaled */
+                                                NULL); /* unnamed event object */
+
+  return handle_data;
+}
+
+static void
+pipe_handle_data_free (PipeHandleData *data)
+{
+  if (data->handle != INVALID_HANDLE_VALUE)
+    CloseHandle (data->handle);
+
+  CloseHandle (data->overlapped.hEvent);
+  g_clear_error (&data->error);
+  g_slice_free (PipeHandleData, data);
+}
+
 static void
 wing_named_pipe_listener_finalize (GObject *object)
 {
@@ -151,75 +176,73 @@ wing_named_pipe_listener_add_named_pipe (WingNamedPipeListener  *listener,
                                          GError                **error)
 {
   WingNamedPipeListenerPrivate *priv;
-  gunichar2 *pipe_namew;
-  PipeData *pipe_data;
-  HANDLE handle;
 
   g_return_val_if_fail (WING_IS_NAMED_PIPE_LISTENER (listener), FALSE);
   g_return_val_if_fail (pipe_name != NULL, FALSE);
 
   priv = wing_named_pipe_listener_get_instance_private (listener);
 
-  pipe_namew = g_utf8_to_utf16 (pipe_name, -1, NULL, NULL, NULL);
-
-  handle = CreateNamedPipeW (pipe_namew,
-                             PIPE_ACCESS_DUPLEX |
-                             FILE_FLAG_OVERLAPPED,
-                             PIPE_TYPE_BYTE |
-                             PIPE_READMODE_BYTE |
-                             PIPE_WAIT,
-                             PIPE_UNLIMITED_INSTANCES,
-                             DEFAULT_PIPE_BUF_SIZE,
-                             DEFAULT_PIPE_BUF_SIZE,
-                             0, NULL);
-  g_free (pipe_namew);
-
-  if (handle == INVALID_HANDLE_VALUE)
+  g_ptr_array_add (priv->named_pipes,
+                   pipe_data_new (pipe_name, source_object));
+
+  return TRUE;
+}
+
+static GList *
+add_sources (GPtrArray             *handle_data_array,
+             WingHandleSourceFunc   callback,
+             gpointer               callback_data,
+             GCancellable          *cancellable,
+             GMainContext          *context)
+{
+  PipeHandleData *data;
+  GSource *source;
+  GList *sources;
+  guint i;
+
+  sources = NULL;
+  for (i = 0; i < handle_data_array->len; i++)
     {
-      int errsv = GetLastError ();
-      gchar *emsg = g_win32_error_message (errsv);
+      data = handle_data_array->pdata[i];
 
-      g_set_error (error,
-                   G_IO_ERROR,
-                   g_io_error_from_win32_error (errsv),
-                   "Error creating named pipe '%s': %s",
-                   pipe_name, emsg);
+      source = _wing_handle_create_source (data->overlapped.hEvent,
+                                           cancellable);
+      g_source_set_callback (source,
+                             (GSourceFunc) callback,
+                             callback_data, NULL);
+      g_source_attach (source, context);
 
-      g_free (emsg);
-      return FALSE;
+      sources = g_list_prepend (sources, source);
     }
 
-  pipe_data = pipe_data_new (pipe_name, handle, source_object);
+  return sources;
+}
 
-  if (!ConnectNamedPipe (handle, &pipe_data->overlapped))
+static void
+free_sources (GList *sources)
+{
+  GSource *source;
+  while (sources != NULL)
     {
-      switch (GetLastError ())
-      {
-      case ERROR_IO_PENDING:
-        break;
-      case ERROR_PIPE_CONNECTED:
-        pipe_data->already_connected = TRUE;
-        break;
-      default:
-        {
-          int errsv = GetLastError ();
-          gchar *emsg = g_win32_error_message (errsv);
-
-          g_set_error (error,
-                       G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
-                       "Failed to connect named pipe '%s': %s",
-                       pipe_name, emsg);
-          g_free (emsg);
-          pipe_data_free (pipe_data);
-
-          return FALSE;
-        }
-      }
+      source = sources->data;
+      sources = g_list_delete_link (sources, sources);
+      g_source_destroy (source);
+      g_source_unref (source);
     }
+}
 
-  g_ptr_array_add (priv->named_pipes, pipe_data);
+typedef struct
+{
+  GPtrArray *handle_data_array;
+  GList *sources;
+} ConnectReadyData;
 
-  return TRUE;
+static void
+free_connect_ready_data (ConnectReadyData *data)
+{
+  g_ptr_array_free (data->handle_data_array, TRUE);
+  free_sources (data->sources);
+  g_slice_free (ConnectReadyData, data);
 }
 
 static gboolean
@@ -227,29 +250,26 @@ connect_ready (HANDLE   handle,
                gpointer user_data)
 {
   GTask *task = user_data;
-  WingNamedPipeListener *listener = g_task_get_source_object (task);
-  WingNamedPipeListenerPrivate *priv;
-  PipeData *pipe_data = NULL;
+  ConnectReadyData *data = g_task_get_task_data (task);
+  PipeHandleData *handle_data = NULL;
   gulong cbret;
   guint i;
 
-  priv = wing_named_pipe_listener_get_instance_private (listener);
-
-  for (i = 0; i < priv->named_pipes->len; i++)
+  for (i = 0; i < data->handle_data_array->len; i++)
     {
-      PipeData *pdata;
+      PipeHandleData *hdata;
 
-      pdata = priv->named_pipes->pdata[i];
-      if (pdata->overlapped.hEvent == handle)
+      hdata = data->handle_data_array->pdata[i];
+      if (hdata->overlapped.hEvent == handle)
         {
-          pipe_data = pdata;
+          handle_data = hdata;
           break;
         }
     }
 
-  g_return_val_if_fail (pipe_data != NULL, FALSE);
+  g_return_val_if_fail (handle_data != NULL, FALSE);
 
-  if (!GetOverlappedResult (pipe_data->handle, &pipe_data->overlapped, &cbret, FALSE))
+  if (!GetOverlappedResult (handle_data->handle, &handle_data->overlapped, &cbret, FALSE))
     {
       int errsv = GetLastError ();
       gchar *emsg = g_win32_error_message (errsv);
@@ -265,16 +285,18 @@ connect_ready (HANDLE   handle,
     {
       WingNamedPipeConnection *connection;
 
-      if (pipe_data->source_object != NULL)
+      if (handle_data->pdata->source_object != NULL)
         g_object_set_qdata_full (G_OBJECT (task),
                                  source_quark,
-                                 g_object_ref (pipe_data->source_object),
+                                 g_object_ref (handle_data->pdata->source_object),
                                  g_object_unref);
 
       connection = g_object_new (WING_TYPE_NAMED_PIPE_CONNECTION,
-                                 "handle", pipe_data->handle,
-                                 "close-handle", FALSE,
+                                 "handle", handle_data->handle,
+                                 "close-handle", TRUE,
                                  NULL);
+      handle_data->handle = INVALID_HANDLE_VALUE;
+
       g_task_return_pointer (task, connection, g_object_unref);
     }
 
@@ -283,56 +305,10 @@ connect_ready (HANDLE   handle,
   return FALSE;
 }
 
-static GList *
-add_sources (WingNamedPipeListener *listener,
-             WingHandleSourceFunc   callback,
-             gpointer               callback_data,
-             GCancellable          *cancellable,
-             GMainContext          *context)
-{
-  WingNamedPipeListenerPrivate *priv;
-  PipeData *data;
-  GSource *source;
-  GList *sources;
-  guint i;
-
-  priv = wing_named_pipe_listener_get_instance_private (listener);
-
-  sources = NULL;
-  for (i = 0; i < priv->named_pipes->len; i++)
-    {
-      data = priv->named_pipes->pdata[i];
-
-      source = _wing_handle_create_source (data->overlapped.hEvent,
-                                           cancellable);
-      g_source_set_callback (source,
-                             (GSourceFunc) callback,
-                             callback_data, NULL);
-      g_source_attach (source, context);
-
-      sources = g_list_prepend (sources, source);
-    }
-
-  return sources;
-}
-
-static void
-free_sources (GList *sources)
-{
-  GSource *source;
-  while (sources != NULL)
-    {
-      source = sources->data;
-      sources = g_list_delete_link (sources, sources);
-      g_source_destroy (source);
-      g_source_unref (source);
-    }
-}
-
 struct AcceptData {
-  WingNamedPipeListener *listener;
   GMainLoop *loop;
-  PipeData *pipe_data;
+  GPtrArray *handle_data_array;
+  PipeHandleData *handle_data;
 };
 
 static gboolean
@@ -340,25 +316,22 @@ accept_callback (HANDLE   handle,
                  gpointer user_data)
 {
   struct AcceptData *data = user_data;
-  WingNamedPipeListenerPrivate *priv;
-  PipeData *pipe_data = NULL;
+  PipeHandleData *handle_data = NULL;
   guint i;
 
-  priv = wing_named_pipe_listener_get_instance_private (data->listener);
-
-  for (i = 0; i < priv->named_pipes->len; i++)
+  for (i = 0; i < data->handle_data_array->len; i++)
     {
-      PipeData *pdata;
+      PipeHandleData *hdata;
 
-      pdata = priv->named_pipes->pdata[i];
-      if (pdata->overlapped.hEvent == handle)
+      hdata = data->handle_data_array->pdata[i];
+      if (hdata->overlapped.hEvent == handle)
         {
-          pipe_data = pdata;
+          handle_data = hdata;
           break;
         }
     }
 
-  data->pipe_data = pipe_data;
+  data->handle_data = handle_data;
   g_main_loop_quit (data->loop);
 
   return TRUE;
@@ -367,23 +340,114 @@ accept_callback (HANDLE   handle,
 /* Check if any of the named pipes is already connected
  * and pick the the first one.
  */
-static PipeData *
-find_first_connected (WingNamedPipeListener *listener)
+static PipeHandleData *
+find_first_connected (GPtrArray *handle_data_array)
+{
+  guint i;
+
+  for (i = 0; i < handle_data_array->len; i++)
+    {
+      PipeHandleData *handle_data = handle_data_array->pdata[i];
+
+      if (handle_data->already_connected)
+        return handle_data;
+    }
+
+  return NULL;
+}
+
+static gboolean
+get_all_pipes_with_error (GPtrArray *handle_data_array)
+{
+  guint i;
+
+  for (i = 0; i < handle_data_array->len; i++)
+    {
+      PipeHandleData *handle_data = handle_data_array->pdata[i];
+
+      if (handle_data->error == NULL)
+        return FALSE;
+    }
+
+  return TRUE;
+}
+
+static GPtrArray *
+connect_pipes (WingNamedPipeListener *listener)
 {
   WingNamedPipeListenerPrivate *priv;
+  GPtrArray *handle_data_array;
   guint i;
 
   priv = wing_named_pipe_listener_get_instance_private (listener);
 
+  handle_data_array = g_ptr_array_new_with_free_func ((GDestroyNotify) pipe_handle_data_free);
+
   for (i = 0; i < priv->named_pipes->len; i++)
     {
       PipeData *pdata = priv->named_pipes->pdata[i];
+      gunichar2 *pipe_namew;
+      HANDLE handle;
+      PipeHandleData *handle_data;
+
+      handle_data = pipe_handle_data_new (pdata);
+      g_ptr_array_add (handle_data_array, handle_data);
+
+      pipe_namew = g_utf8_to_utf16 (pdata->pipe_name, -1, NULL, NULL, NULL);
+
+      handle = CreateNamedPipeW (pipe_namew,
+                                 PIPE_ACCESS_DUPLEX |
+                                 FILE_FLAG_OVERLAPPED,
+                                 PIPE_TYPE_BYTE |
+                                 PIPE_READMODE_BYTE |
+                                 PIPE_WAIT,
+                                 PIPE_UNLIMITED_INSTANCES,
+                                 DEFAULT_PIPE_BUF_SIZE,
+                                 DEFAULT_PIPE_BUF_SIZE,
+                                 0, NULL);
+      g_free (pipe_namew);
+
+      if (handle == INVALID_HANDLE_VALUE)
+        {
+          int errsv = GetLastError ();
+          gchar *emsg = g_win32_error_message (errsv);
 
-      if (pdata->already_connected)
-        return pdata;
+          handle_data->error = g_error_new (G_IO_ERROR,
+                                            g_io_error_from_win32_error (errsv),
+                                            "Error creating named pipe '%s': %s",
+                                            pdata->pipe_name, emsg);
+          g_free (emsg);
+
+          continue;
+        }
+
+      handle_data->handle = handle;
+
+      if (!ConnectNamedPipe (handle, &handle_data->overlapped))
+        {
+          switch (GetLastError ())
+          {
+          case ERROR_IO_PENDING:
+            break;
+          case ERROR_PIPE_CONNECTED:
+            handle_data->already_connected = TRUE;
+            break;
+          default:
+            {
+              int errsv = GetLastError ();
+              gchar *emsg = g_win32_error_message (errsv);
+
+              handle_data->error = g_error_new (G_IO_ERROR,
+                                                g_io_error_from_win32_error (errsv),
+                                                "Failed to connect named pipe '%s': %s",
+                                                pdata->pipe_name, emsg);
+              g_free (emsg);
+            }
+          }
+        }
     }
 
-  return NULL;
+  return handle_data_array;
 }
 
 /**
@@ -415,31 +479,42 @@ wing_named_pipe_listener_accept (WingNamedPipeListener  *listener,
                                  GError                **error)
 {
   WingNamedPipeListenerPrivate *priv;
-  PipeData *pipe_data = NULL;
+  PipeHandleData *handle_data = NULL;
   WingNamedPipeConnection *connection = NULL;
+  GPtrArray *handle_data_array;
 
   g_return_val_if_fail (WING_IS_NAMED_PIPE_LISTENER (listener), NULL);
 
   priv = wing_named_pipe_listener_get_instance_private (listener);
 
-  if (priv->named_pipes->len == 1)
+  handle_data_array = connect_pipes (listener);
+  if (get_all_pipes_with_error (handle_data_array))
+    {
+      /* Return the first error */
+      handle_data = handle_data_array->pdata[0];
+      *error = g_error_copy (handle_data->error);
+      g_ptr_array_free (handle_data_array, TRUE);
+      return NULL;
+    }
+
+  if (handle_data_array->len == 1)
     {
       gboolean success;
 
-      pipe_data = priv->named_pipes->pdata[0];
-      success = pipe_data->already_connected;
+      handle_data = handle_data_array->pdata[0];
+      success = handle_data->already_connected;
 
       if (!success)
-        success = WaitForSingleObject (pipe_data->overlapped.hEvent, INFINITE) == WAIT_OBJECT_0;
+        success = WaitForSingleObject (handle_data->overlapped.hEvent, INFINITE) == WAIT_OBJECT_0;
 
       if (!success)
-        pipe_data = NULL;
+        handle_data = NULL;
     }
   else
     {
-      pipe_data = find_first_connected (listener);
+      handle_data = find_first_connected (handle_data_array);
 
-      if (pipe_data == NULL)
+      if (handle_data == NULL)
         {
           GList *sources;
           struct AcceptData data;
@@ -450,29 +525,33 @@ wing_named_pipe_listener_accept (WingNamedPipeListener  *listener,
 
           loop = g_main_loop_new (priv->main_context, FALSE);
           data.loop = loop;
-          data.listener = listener;
+          data.handle_data_array = handle_data_array;
 
-          sources = add_sources (listener,
+          sources = add_sources (handle_data_array,
                                  accept_callback,
                                  &data,
                                  cancellable,
                                  priv->main_context);
           g_main_loop_run (loop);
-          pipe_data = data.pipe_data;
+          handle_data = data.handle_data;
           free_sources (sources);
           g_main_loop_unref (loop);
         }
     }
 
-  if (pipe_data != NULL)
+  if (handle_data != NULL)
     {
       connection = g_object_new (WING_TYPE_NAMED_PIPE_CONNECTION,
-                                 "handle", pipe_data->handle,
-                                 "close-handle", FALSE,
+                                 "handle", handle_data->handle,
+                                 "close-handle", TRUE,
                                  NULL);
 
+      handle_data->handle = INVALID_HANDLE_VALUE;
+
       if (source_object)
-        *source_object = pipe_data->source_object;
+        *source_object = handle_data->pdata->source_object;
+
+      g_ptr_array_free (handle_data_array, TRUE);
     }
 
   return connection;
@@ -500,44 +579,62 @@ wing_named_pipe_listener_accept_async (WingNamedPipeListener *listener,
                                        gpointer               user_data)
 {
   WingNamedPipeListenerPrivate *priv;
-  PipeData *pipe_data;
+  GPtrArray *handle_data_array;
+  PipeHandleData *handle_data;
   GTask *task;
-  GList *sources;
+  ConnectReadyData *data;
   guint i;
 
   task = g_task_new (listener, cancellable, callback, user_data);
 
   priv = wing_named_pipe_listener_get_instance_private (listener);
 
-  pipe_data = find_first_connected (listener);
+  handle_data_array = connect_pipes (listener);
+  if (get_all_pipes_with_error (handle_data_array))
+    {
+      /* Return the first error */
+      handle_data = handle_data_array->pdata[0];
+      g_task_return_error (task, g_error_copy (handle_data->error));
+      g_object_unref (task);
+      g_ptr_array_free (handle_data_array, TRUE);
+
+      return;
+    }
 
-  if (pipe_data != NULL)
+  handle_data = find_first_connected (handle_data_array);
+
+  if (handle_data != NULL)
     {
       WingNamedPipeConnection *connection;
 
-      if (pipe_data->source_object)
+      if (handle_data->pdata->source_object)
         g_object_set_qdata_full (G_OBJECT (task),
                                  source_quark,
-                                 g_object_ref (pipe_data->source_object),
+                                 g_object_ref (handle_data->pdata->source_object),
                                  g_object_unref);
 
       connection = g_object_new (WING_TYPE_NAMED_PIPE_CONNECTION,
-                                 "handle", pipe_data->handle,
-                                 "close-handle", FALSE,
+                                 "handle", handle_data->handle,
+                                 "close-handle", TRUE,
                                  NULL);
 
+      handle_data->handle = INVALID_HANDLE_VALUE;
+
       g_task_return_pointer (task, connection, g_object_unref);
       g_object_unref (task);
+      g_ptr_array_free (handle_data_array, TRUE);
 
       return;
     }
 
-  sources = add_sources (listener,
-                         connect_ready,
-                         task,
-                         cancellable,
-                         g_main_context_get_thread_default ());
-  g_task_set_task_data (task, sources, (GDestroyNotify) free_sources);
+  data = g_slice_new0 (ConnectReadyData);
+  data->handle_data_array = handle_data_array;
+  data->sources = add_sources (handle_data_array,
+                               connect_ready,
+                               task,
+                               cancellable,
+                               g_main_context_get_thread_default ());
+  g_task_set_task_data (task, data, (GDestroyNotify) free_connect_ready_data);
 }
 
 /**


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]