Attachment 93677 Details for Bug 60847 – merged patch

[patch] merged patch

0001-PolkitAgentSession-fix-race-between-child-and-io-wat.patch (text/plain), 3.17 KB, created by Colin Walters on 2014-02-08 21:30:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2014-02-08 21:30:30 UTC

Size: 3.17 KB

patch

obsolete

>From 928d235b72678c24d5aff5b0b4738e0a4def4ac4 Mon Sep 17 00:00:00 2001
>From: Rui Matos <tiagomatos@gmail.com>
>Date: Thu, 6 Feb 2014 18:41:18 +0100
>Subject: [PATCH] PolkitAgentSession: fix race between child and io watches
>
>The helper flushes and fdatasyncs stdout and stderr before terminating
>but this doesn't guarantee that our io watch is called before our
>child watch. This means that we can end up with a successful return
>from the helper which we still report as a failure.
>
>To fix this, instead of reporting a failure from the child watch
>handler, we'll just unset child_pid.
>
>In addition, we add G_IO_HUP and G_IO_ERR to the conditions we look
>for in the io watch so that if the child terminates abnormally we
>still run the io watch handler which will complete the session
>reporting a failure.
>---
> src/polkitagent/polkitagentsession.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c
>index 1c7a2dc..0d35e95 100644
>--- a/src/polkitagent/polkitagentsession.c
>+++ b/src/polkitagent/polkitagentsession.c
>@@ -444,9 +444,7 @@ child_watch_func (GPid     pid,
>                WEXITSTATUS(status));
>     }
> 
>-  /* kill all the watches we have set up, except for the child since it has exited already */
>   session->child_pid = 0;
>-  complete_session (session, FALSE);
> }
> 
> static gboolean
>@@ -470,15 +468,24 @@ io_watch_have_data (GIOChannel    *channel,
>       goto out;
>     }
> 
>+  if (!(condition & G_IO_IN))
>+    {
>+      complete_session (session, FALSE);
>+      goto out;
>+    }
>+
>   g_io_channel_read_line (channel,
>                           &line,
>                           NULL,
>                           NULL,
>                           &error);
>-  if (error != NULL)
>+  if (error != NULL || line == NULL)
>     {
>-      g_warning ("Error reading line from helper: %s", error->message);
>-      g_error_free (error);
>+      /* In case we get just G_IO_HUP, line is NULL but error is
>+         unset.*/
>+      g_warning ("Error reading line from helper: %s",
>+                 error ? error->message : "nothing to read");
>+      g_clear_error (&error);
> 
>       complete_session (session, FALSE);
>       goto out;
>@@ -540,6 +547,9 @@ io_watch_have_data (GIOChannel    *channel,
>   g_free (line);
>   g_free (unescaped);
> 
>+  if (condition & (G_IO_ERR | G_IO_HUP))
>+    complete_session (session, FALSE);
>+
>   /* keep the IOChannel around */
>   return TRUE;
> }
>@@ -655,7 +665,8 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
>   g_source_attach (session->child_watch_source, g_main_context_get_thread_default ());
> 
>   session->child_stdout_channel = g_io_channel_unix_new (session->child_stdout);
>-  session->child_stdout_watch_source = g_io_create_watch (session->child_stdout_channel, G_IO_IN);
>+  session->child_stdout_watch_source = g_io_create_watch (session->child_stdout_channel,
>+                                                          G_IO_IN | G_IO_ERR | G_IO_HUP);
>   g_source_set_callback (session->child_stdout_watch_source, (GSourceFunc) io_watch_have_data, session, NULL);
>   g_source_attach (session->child_stdout_watch_source, g_main_context_get_thread_default ());
> 
>-- 
>1.8.3.1
>

Actions: View | Splinter Review

Attachments on bug 60847: 93553 | 93662 | 93677

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.