Merge lp:~ted/ubuntu-app-launch/bad-appid-recoverable-error into lp:ubuntu-app-launch/14.04

[Ted Gould]

More elegant handling of bad application id errors

We get various bugs on this, which is good that they're being reported, but there's a bit of noise. Trying to filter out that noise and be a bit more sophisticated on the reporting.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:151
http://jenkins.qa.ubuntu.com/job/upstart-app-launch-ci/256/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-amd64-ci/115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-armhf-ci/115
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-i386-ci/116

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/upstart-app-launch-ci/256/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

* cmdline is leaked.

* Not a blocker for this merge, but as a general comment: we really need to move recoverable-problem.c into some shared location

[Charles Kerr (charlesk)]

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-03-12 00:28:05 +0000
+++ CMakeLists.txt	2014-04-16 18:51:37 +0000
@@ -83,7 +83,7 @@
 ####################
 
 add_lttng_gen_tp(NAME desktop-exec-trace)
-add_executable(desktop-exec desktop-exec.c desktop-exec-trace.c)
+add_executable(desktop-exec desktop-exec.c desktop-exec-trace.c recoverable-problem.c)
 set_target_properties(desktop-exec PROPERTIES OUTPUT_NAME "desktop-exec")
 target_link_libraries(desktop-exec helpers ${LTTNG_LIBRARIES})
 install(TARGETS desktop-exec RUNTIME DESTINATION "${pkglibexecdir}")
 
=== modified file 'desktop-exec.c'
--- desktop-exec.c	2014-02-07 14:56:27 +0000
+++ desktop-exec.c	2014-04-16 18:51:37 +0000
@@ -25,6 +25,7 @@
 
 #include "helpers.h"
 #include "desktop-exec-trace.h"
+#include "recoverable-problem.h"
 
 int
 main (int argc, char * argv[])
@@ -65,7 +66,47 @@
 	GKeyFile * keyfile = keyfile_for_appid(app_id, &desktopfilename);
 
 	if (keyfile == NULL) {
-		g_error("Unable to find keyfile for application '%s'", app_id);
+		g_warning("Unable to find keyfile for application '%s'", app_id);
+
+		const gchar * props[3] = {
+			"AppId", NULL,
+			NULL
+		};
+		props[1] = app_id;
+
+		GPid pid = 0;
+		const gchar * launcher_pid = g_getenv("APP_LAUNCHER_PID");
+		if (launcher_pid != NULL) {
+			pid = atoi(launcher_pid);
+		}
+
+		/* Checking to see if we're using the command line tool to create
+		   the appid. Chances are in that case it's a user error, and we
+		   don't need to automatically record it, the user mistyped. */
+		gboolean debugtool = FALSE;
+		if (pid != 0) {
+			gchar * cmdpath = g_strdup_printf("/proc/%d/cmdline", pid);
+			gchar * cmdline = NULL;
+
+			if (g_file_get_contents(cmdpath, &cmdline, NULL, NULL)) {
+				if (g_strstr_len(cmdline, -1, "upstart-app-launch") != NULL) {
+					debugtool = TRUE;
+				}
+
+				g_free(cmdline);
+			} else {
+				/* The caller has already exited, probably a debug tool */
+				debugtool = TRUE;
+			}
+
+			g_free(cmdpath);
+		}
+
+		if (!debugtool) {
+			report_recoverable_problem("upstart-app-launch-invalid-appid", pid, TRUE, props);
+		} else {
+			g_debug("Suppressing appid recoverable error for debug tool");
+		}
 		return 1;
 	}
 
 
=== modified file 'libupstart-app-launch/upstart-app-launch.c'
--- libupstart-app-launch/upstart-app-launch.c	2014-03-21 17:56:15 +0000
+++ libupstart-app-launch/upstart-app-launch.c	2014-04-16 18:51:37 +0000
@@ -228,6 +228,7 @@
 	g_variant_builder_open(&builder, G_VARIANT_TYPE_ARRAY);
 
 	g_variant_builder_add_value(&builder, g_variant_new_take_string(g_strdup_printf("APP_ID=%s", appid)));
+	g_variant_builder_add_value(&builder, g_variant_new_take_string(g_strdup_printf("APP_LAUNCHER_PID=%d", getpid())));
 
 	if (uris != NULL) {
 		gchar * urisjoin = app_uris_string(uris);
 
=== added file 'recoverable-problem.c'
--- recoverable-problem.c	1970-01-01 00:00:00 +0000
+++ recoverable-problem.c	2014-04-16 18:51:37 +0000
@@ -0,0 +1,165 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Ted Gould <[e-mail]>
+ */
+
+#include "recoverable-problem.h"
+#include <glib/gstdio.h>
+#include <string.h>
+#include <errno.h>
+
+/* Helpers to ensure we write nicely */
+static void 
+write_string (int          fd,
+              const gchar *string)
+{
+	int res; 
+	do
+		res = write (fd, string, strlen (string));
+	while (G_UNLIKELY (res == -1 && errno == EINTR));
+}
+
+/* Make NULLs fast and fun! */
+static void 
+write_null (int fd)
+{
+	int res; 
+	do
+		res = write (fd, "", 1);
+	while (G_UNLIKELY (res == -1 && errno == EINTR));
+}
+
+/* Child watcher */
+static gboolean
+apport_child_watch (GPid pid, gint status, gpointer user_data)
+{
+	g_main_loop_quit((GMainLoop *)user_data);
+	return FALSE;
+}
+
+static gboolean
+apport_child_timeout (gpointer user_data)
+{
+	g_warning("Recoverable Error Reporter Timeout");
+	g_main_loop_quit((GMainLoop *)user_data);
+	return FALSE;
+}
+
+/* Code to report an error */
+void
+report_recoverable_problem (const gchar * signature, GPid report_pid, gboolean wait, const gchar * additional_properties[])
+{
+	GError * error = NULL;
+	gint error_stdin = 0;
+	GPid pid = 0;
+	gchar * pid_str = NULL;
+	gchar ** argv = NULL;
+	gchar * argv_nopid[2] = {
+		"/usr/share/apport/recoverable_problem",
+		NULL
+	};
+	gchar * argv_pid[4] = {
+		"/usr/share/apport/recoverable_problem",
+		"-p",
+		NULL, /* put pid_str when allocated here */
+		NULL
+	};
+
+
+	argv = (gchar **)argv_nopid;
+
+	if (report_pid != 0) {
+		pid_str = g_strdup_printf("%d", report_pid);
+		argv_pid[2] = pid_str;
+		argv = (gchar**)argv_pid;
+	}
+
+	GSpawnFlags flags = G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL;
+	if (wait) {
+		flags |= G_SPAWN_DO_NOT_REAP_CHILD;
+	}
+
+	g_spawn_async_with_pipes(NULL, /* cwd */
+		argv,
+		NULL, /* envp */
+		flags,
+		NULL, NULL, /* child setup func */
+		&pid,
+		&error_stdin,
+		NULL, /* stdout */
+		NULL, /* stderr */
+		&error);
+
+	if (error != NULL) {
+		g_warning("Unable to report a recoverable error: %s", error->message);
+		g_error_free(error);
+	}
+
+	gboolean first = TRUE;
+
+	if (error_stdin != 0 && signature != NULL) {
+		write_string(error_stdin, "DuplicateSignature");
+		write_null(error_stdin);
+		write_string(error_stdin, signature);
+
+		first = FALSE;
+	}
+
+	if (error_stdin != 0 && additional_properties != NULL) {
+		gint i;
+		for (i = 0; additional_properties[i] != NULL; i++) {
+			if (!first) {
+				write_null(error_stdin);
+			} else {
+				first = FALSE;
+			}
+
+			write_string(error_stdin, additional_properties[i]);
+		}
+	}
+
+	if (error_stdin != 0) {
+		close(error_stdin);
+	}
+
+	if (wait && pid != 0) {
+		GSource * child_source, * timeout_source;
+		GMainContext * context = g_main_context_new();
+		GMainLoop * loop = g_main_loop_new(context, FALSE);
+
+		child_source = g_child_watch_source_new(pid);
+		g_source_attach(child_source, context);
+		g_source_set_callback(child_source, (GSourceFunc)apport_child_watch, loop, NULL);
+
+		timeout_source = g_timeout_source_new_seconds(5);
+		g_source_attach(timeout_source, context);
+		g_source_set_callback(timeout_source, apport_child_timeout, loop, NULL);
+
+		g_main_loop_run(loop);
+
+		g_source_destroy(timeout_source);
+		g_source_destroy(child_source);
+		g_main_loop_unref(loop);
+		g_main_context_unref(context);
+
+		g_spawn_close_pid(pid);
+	}
+
+	g_free(pid_str);
+
+	return;
+}
 
=== added file 'recoverable-problem.h'
--- recoverable-problem.h	1970-01-01 00:00:00 +0000
+++ recoverable-problem.h	2014-04-16 18:51:37 +0000
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *     Ted Gould <[e-mail]>
+ */
+
+#include <gio/gio.h>
+
+void    report_recoverable_problem    (const gchar *   signature,
+                                       GPid            report_pid,
+                                       gboolean        wait,
+                                       const gchar *   additional_properties[]);
+