[PATCH 1/2] Fix aborting capability scripts making them zombies processes

[PATCH 1/2] Fix aborting capability scripts making them zombies processes

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

When using G_SPAWN_DO_NOT_REAP_CHILD the watch cannot be removed since it
basically replaces the use of waitpid which does prevent the child
process to became 'zombie' when terminated, a more detailed explanation
can be found in waitpid manpage.
---
 plugins/filesystem.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/plugins/filesystem.c b/plugins/filesystem.c
index f0e5bbe..bf00ac2 100644
--- a/plugins/filesystem.c
+++ b/plugins/filesystem.c
@@ -237,7 +237,7 @@ struct capability_object {
 	int pid;
 	int output;
 	int err;
-	unsigned int watch;
+	gboolean aborted;
 	GString *buffer;
 };
 
@@ -247,10 +247,20 @@ static void script_exited(GPid pid, int status, void *data)
 	char buf[128];
 
 	object->pid = -1;
-	object->watch = 0;
 
 	DBG("pid: %d status: %d", pid, status);
 
+	g_spawn_close_pid(pid);
+
+	/* free the object if aborted */
+	if (object->aborted) {
+		if (object->buffer != NULL)
+			g_string_free(object->buffer, TRUE);
+
+		g_free(object);
+		return;
+	}
+
 	if (WEXITSTATUS(status) != EXIT_SUCCESS) {
 		memset(buf, 0, sizeof(buf));
 		if (read(object->err, buf, sizeof(buf)) > 0)
@@ -258,8 +268,6 @@ static void script_exited(GPid pid, int status, void *data)
 		obex_object_set_io_flags(data, G_IO_ERR, -EPERM);
 	} else
 		obex_object_set_io_flags(data, G_IO_IN, 0);
-
-	g_spawn_close_pid(pid);
 }
 
 static int capability_exec(const char **argv, int *output, int *err)
@@ -321,7 +329,8 @@ static void *capability_open(const char *name, int oflag, mode_t mode,
 	if (object->pid < 0)
 		goto fail;
 
-	object->watch = g_child_watch_add(object->pid, script_exited, object);
+	/* Watch cannot be removed while the process is still running */
+	g_child_watch_add(object->pid, script_exited, object);
 
 done:
 	if (err)
@@ -519,18 +528,17 @@ static int capability_close(void *object)
 	if (obj->pid < 0)
 		goto done;
 
-	if (obj->watch)
-		g_source_remove(obj->watch);
-
-	g_spawn_close_pid(obj->pid);
-
 	DBG("kill: pid %d", obj->pid);
 	err = kill(obj->pid, SIGTERM);
 	if (err < 0) {
 		err = -errno;
 		error("kill: %s (%d)", strerror(-err), -err);
+		goto done;
 	}
 
+	obj->aborted = TRUE;
+	return 0;
+
 done:
 	if (obj->buffer != NULL)
 		g_string_free(obj->buffer, TRUE);
-- 
1.7.0.4

[PATCH 2/2] gwobex: fix misuse of OBEX_SetCustomData/OBEX_GetCustomData

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Those function should only be used in case of custom transport which is
not the case since we are using fd transport here.

To fix that it now uses OBEX_SetUserData/OBEX_GetUserData which are not
associate with any transport in particular.
---
 gwobex/gw-obex.c   |    2 +-
 gwobex/obex-priv.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gwobex/gw-obex.c b/gwobex/gw-obex.c
index e656692..4481212 100644
--- a/gwobex/gw-obex.c
+++ b/gwobex/gw-obex.c
@@ -343,7 +343,7 @@ GwObex *gw_obex_setup_fd(int fd, const gchar *uuid, gint uuid_len,
     ctx->mutex = g_mutex_new();
 #endif
 
-    OBEX_SetCustomData(handle, ctx);
+    OBEX_SetUserData(handle, ctx);
 
     debug("Connecting to OBEX service\n");
     if (!gw_obex_connect(ctx, uuid, uuid_len)) {
diff --git a/gwobex/obex-priv.c b/gwobex/obex-priv.c
index 76a9599..aba7dd7 100644
--- a/gwobex/obex-priv.c
+++ b/gwobex/obex-priv.c
@@ -514,7 +514,7 @@ static void obex_writestream(GwObex *ctx, obex_object_t *object) {
 
 static void obex_event_handler(obex_t *handle, obex_object_t *object, int mode,
                                int event, int obex_cmd, int obex_rsp) {
-    GwObex *ctx = OBEX_GetCustomData(handle);
+    GwObex *ctx = OBEX_GetUserData(handle);
     switch (event) {
         case OBEX_EV_ABORT:
             debug("OBEX_EV_ABORT\n");
-- 
1.7.0.4

Re: [PATCH 2/2] gwobex: fix misuse of OBEX_SetCustomData/OBEX_GetCustomData

[Johan Hedberg]

Hi Luiz,

On Tue, Aug 17, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> Those function should only be used in case of custom transport which is
> not the case since we are using fd transport here.
> 
> To fix that it now uses OBEX_SetUserData/OBEX_GetUserData which are not
> associate with any transport in particular.
> ---
>  gwobex/gw-obex.c   |    2 +-
>  gwobex/obex-priv.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

This one has also been pushed to the obexd upstream tree. Also remember
to update the upstream gwobex tree at gitorious.org.

Johan

Re: [PATCH 1/2] Fix aborting capability scripts making them zombies processes

[Johan Hedberg]

Hi Luiz,

On Tue, Aug 17, 2010, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> 
> When using G_SPAWN_DO_NOT_REAP_CHILD the watch cannot be removed since it
> basically replaces the use of waitpid which does prevent the child
> process to became 'zombie' when terminated, a more detailed explanation
> can be found in waitpid manpage.
> ---
>  plugins/filesystem.c |   28 ++++++++++++++++++----------
>  1 files changed, 18 insertions(+), 10 deletions(-)

Thanks. The patch is now upstream.

Johan