]>
Commit | Line | Data |
---|---|---|
e31e969b JR |
1 | From eb860fd898a6a2bd86c11d245294cd0e8cd4304b Mon Sep 17 00:00:00 2001 |
2 | From: Ryan Lortie <desrt@desrt.ca> | |
3 | Date: Tue, 23 Apr 2013 17:26:48 +0000 | |
4 | Subject: Partially revert "Merge waitpid() from g_spawn_sync into gmain()" | |
5 | ||
6 | This partially reverts commit ce0022933c255313e010b27f977f4ae02aad1e7e. | |
7 | ||
8 | It used to be safe to use g_spawn_sync() from processes that had their | |
9 | own SIGCHLD handler because it simply called wait(). When it was | |
10 | changed to depend on the GLib child watching infrastructure this meant | |
11 | that GLib had to own the SIGCHLD handler. | |
12 | ||
13 | This caused hangs in at least Pidgin. | |
14 | ||
15 | The patch contained two other improvements to the child watch code which | |
16 | we want to keep, so only revert the changes to gspawn itself. | |
17 | ||
18 | https://bugzilla.gnome.org/show_bug.cgi?id=698081 | |
19 | --- | |
20 | (limited to 'glib/gspawn.c') | |
21 | ||
22 | diff --git a/glib/gspawn.c b/glib/gspawn.c | |
23 | index 381ed5c..01cedf6 100644 | |
24 | --- a/glib/gspawn.c | |
25 | +++ b/glib/gspawn.c | |
26 | @@ -47,7 +47,6 @@ | |
27 | ||
28 | #include "genviron.h" | |
29 | #include "gmem.h" | |
30 | -#include "gmain.h" | |
31 | #include "gshell.h" | |
32 | #include "gstring.h" | |
33 | #include "gstrfuncs.h" | |
34 | @@ -207,21 +206,6 @@ read_data (GString *str, | |
35 | } | |
36 | } | |
37 | ||
38 | -typedef struct { | |
39 | - GMainLoop *loop; | |
40 | - gint *status_p; | |
41 | -} SyncWaitpidData; | |
42 | - | |
43 | -static void | |
44 | -on_sync_waitpid (GPid pid, | |
45 | - gint status, | |
46 | - gpointer user_data) | |
47 | -{ | |
48 | - SyncWaitpidData *data = user_data; | |
49 | - *(data->status_p) = status; | |
50 | - g_main_loop_quit (data->loop); | |
51 | -} | |
52 | - | |
53 | /** | |
54 | * g_spawn_sync: | |
55 | * @working_directory: (allow-none): child's current working directory, or %NULL to inherit parent's | |
56 | @@ -277,7 +261,6 @@ g_spawn_sync (const gchar *working_directory, | |
57 | GString *errstr = NULL; | |
58 | gboolean failed; | |
59 | gint status; | |
60 | - SyncWaitpidData waitpid_data; | |
61 | ||
62 | g_return_val_if_fail (argv != NULL, FALSE); | |
63 | g_return_val_if_fail (!(flags & G_SPAWN_DO_NOT_REAP_CHILD), FALSE); | |
64 | @@ -410,32 +393,45 @@ g_spawn_sync (const gchar *working_directory, | |
65 | close_and_invalidate (&outpipe); | |
66 | if (errpipe >= 0) | |
67 | close_and_invalidate (&errpipe); | |
68 | - | |
69 | - /* Now create a temporary main context and loop, with just one | |
70 | - * waitpid source. We used to invoke waitpid() directly here, but | |
71 | - * this way we unify with the worker thread in gmain.c. | |
72 | + | |
73 | + /* Wait for child to exit, even if we have | |
74 | + * an error pending. | |
75 | */ | |
76 | - { | |
77 | - GMainContext *context; | |
78 | - GMainLoop *loop; | |
79 | - GSource *source; | |
80 | - | |
81 | - context = g_main_context_new (); | |
82 | - loop = g_main_loop_new (context, TRUE); | |
83 | + again: | |
84 | + | |
85 | + ret = waitpid (pid, &status, 0); | |
86 | ||
87 | - waitpid_data.loop = loop; | |
88 | - waitpid_data.status_p = &status; | |
89 | - | |
90 | - source = g_child_watch_source_new (pid); | |
91 | - g_source_set_callback (source, (GSourceFunc)on_sync_waitpid, &waitpid_data, NULL); | |
92 | - g_source_attach (source, context); | |
93 | - g_source_unref (source); | |
94 | - | |
95 | - g_main_loop_run (loop); | |
96 | + if (ret < 0) | |
97 | + { | |
98 | + if (errno == EINTR) | |
99 | + goto again; | |
100 | + else if (errno == ECHILD) | |
101 | + { | |
102 | + if (exit_status) | |
103 | + { | |
104 | + g_warning ("In call to g_spawn_sync(), exit status of a child process was requested but ECHILD was received by waitpid(). Most likely the process is ignoring SIGCHLD, or some other thread is invoking waitpid() with a nonpositive first argument; either behavior can break applications that use g_spawn_sync either directly or indirectly."); | |
105 | + } | |
106 | + else | |
107 | + { | |
108 | + /* We don't need the exit status. */ | |
109 | + } | |
110 | + } | |
111 | + else | |
112 | + { | |
113 | + if (!failed) /* avoid error pileups */ | |
114 | + { | |
115 | + int errsv = errno; | |
116 | ||
117 | - g_main_context_unref (context); | |
118 | - g_main_loop_unref (loop); | |
119 | - } | |
120 | + failed = TRUE; | |
121 | + | |
122 | + g_set_error (error, | |
123 | + G_SPAWN_ERROR, | |
124 | + G_SPAWN_ERROR_READ, | |
125 | + _("Unexpected error in waitpid() (%s)"), | |
126 | + g_strerror (errsv)); | |
127 | + } | |
128 | + } | |
129 | + } | |
130 | ||
131 | if (failed) | |
132 | { | |
133 | -- | |
134 | cgit v0.9.1 |