abrt-applet: set SIGCHLD to SIG_IGN to prevent zombies from gui

Also, running abrt-gui via popen is definiely wrong. We were leaking a FILE!
fixed now.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

4 files changed, 70 insertions, 64 deletions

diff --git a/src/Applet/Applet.cpp b/src/Applet/Applet.cpp
index 3ae9b4a..adcfb47 100644
--- a/src/Applet/Applet.cpp
+++ b/src/Applet/Applet.cpp
@@ -16,7 +16,7 @@
     with this program; if not, write to the Free Software Foundation, Inc., 
     51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. 
     */
-    
+
 #include "CCApplet.h"
 #include <iostream>
 #include <dbus/dbus-shared.h>
@@ -42,6 +42,9 @@ int main(int argc, char **argv)
     gdk_threads_init();
     gdk_threads_enter();
     gtk_init(&argc,&argv);
+    /* prevent zombies when we spawn abrt-gui */
+    signal(SIGCHLD, SIG_IGN);
+
     /* move to the DBusClient::connect */
     DBus::Glib::BusDispatcher dispatcher;
     /* this should bind the dispatcher with mainloop */
@@ -60,7 +63,7 @@ int main(int argc, char **argv)
         //applet is not running, so claim the name on the session bus
         session.request_name("com.redhat.abrt.applet");
     }
-    
+
     DBus::Connection conn = DBus::Connection::SystemBus();
     applet = new CApplet(conn, CC_DBUS_PATH, CC_DBUS_NAME);
     applet->ConnectCrashHandler(crash_notify_cb);
@@ -69,6 +72,7 @@ int main(int argc, char **argv)
         std::cout << "Daemon is not running" << std::endl;
         applet->Disable("Daemon is not running");
     }
+
     gtk_main();
     gdk_threads_leave();
     delete applet;
diff --git a/src/Applet/CCApplet.cpp b/src/Applet/CCApplet.cpp
index eede5d1..90df3fc 100644
--- a/src/Applet/CCApplet.cpp
+++ b/src/Applet/CCApplet.cpp
@@ -1,22 +1,23 @@
-/* 
-    Copyright (C) 2009  Jiri Moskovcak (jmoskovc@redhat.com) 
-    Copyright (C) 2009  RedHat inc. 
- 
-    This program is free software; you can redistribute it and/or modify 
-    it under the terms of the GNU General Public License as published by 
-    the Free Software Foundation; either version 2 of the License, or 
-    (at your option) any later version. 
- 
-    This program is distributed in the hope that it will be useful, 
-    but WITHOUT ANY WARRANTY; without even the implied warranty of 
-    MERCHANTABILITY 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, write to the Free Software Foundation, Inc., 
-    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. 
+/*
+    Copyright (C) 2009  Jiri Moskovcak (jmoskovc@redhat.com)
+    Copyright (C) 2009  RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY 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, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
     */
 
+#include "abrtlib.h"
 #include "CCApplet.h"
 #include <iostream>
 #include <cstdarg>
@@ -30,20 +31,19 @@ CApplet::CApplet(DBus::Connection &connection, const char *path, const char *nam
 : DBus::ObjectProxy(connection, path, name)
 {
     m_pDaemonWatcher = new DaemonWatcher(connection, DBUS_SERVICE_PATH, DBUS_SERVICE_NAME);
-    m_pDaemonWatcher->ConnectStateChangeHandler(DaemonStateChange_cb,this);
-    m_pStatusIcon =  gtk_status_icon_new_from_stock(GTK_STOCK_DIALOG_WARNING);
+    m_pDaemonWatcher->ConnectStateChangeHandler(DaemonStateChange_cb, this);
+    m_pStatusIcon = gtk_status_icon_new_from_stock(GTK_STOCK_DIALOG_WARNING);
     m_bDaemonRunning = true;
-    char notify_title[5] = "ABRT";
-    notify_init(notify_title);
-    m_pNotification =  notify_notification_new_with_status_icon("Warning!",NULL, NULL,m_pStatusIcon);
-    notify_notification_set_urgency(m_pNotification,NOTIFY_URGENCY_CRITICAL);
+    notify_init("ABRT");
+    m_pNotification = notify_notification_new_with_status_icon("Warning!", NULL, NULL, m_pStatusIcon);
+    notify_notification_set_urgency(m_pNotification, NOTIFY_URGENCY_CRITICAL);
     notify_notification_set_timeout(m_pNotification, 5000);
-    gtk_status_icon_set_visible(m_pStatusIcon,FALSE);
+    gtk_status_icon_set_visible(m_pStatusIcon, FALSE);
     // LMB click
     //TODO add some actions!
-    g_signal_connect(G_OBJECT(m_pStatusIcon),"activate",GTK_SIGNAL_FUNC(CApplet::OnAppletActivate_CB), this);
-    g_signal_connect(G_OBJECT(m_pStatusIcon),"popup_menu",GTK_SIGNAL_FUNC(CApplet::OnMenuPopup_cb), this);
-    SetIconTooltip("Pending events: %i",m_mapEvents.size());
+    g_signal_connect(G_OBJECT(m_pStatusIcon), "activate", GTK_SIGNAL_FUNC(CApplet::OnAppletActivate_CB), this);
+    g_signal_connect(G_OBJECT(m_pStatusIcon), "popup_menu", GTK_SIGNAL_FUNC(CApplet::OnMenuPopup_cb), this);
+    SetIconTooltip("Pending events: %i", m_mapEvents.size());
 }
 
 CApplet::~CApplet()
@@ -111,18 +111,23 @@ void CApplet::SetIconTooltip(const char *format, ...)
         gtk_status_icon_set_tooltip_text(m_pStatusIcon,"Error while setting the tooltip!");
     }
     delete[] buf;
-    
 }
 
 void CApplet::OnAppletActivate_CB(GtkStatusIcon *status_icon,gpointer user_data)
 {
     CApplet *applet = (CApplet *)user_data;
-    FILE *gui = NULL;
-    //FIXME - use fork+exec and absolute paths? or dbus?
-    if(applet->m_bDaemonRunning)
+    if (applet->m_bDaemonRunning)
     {
-        gui = popen((std::string(BIN_DIR) + "/abrt-gui").c_str(),"r");
-        gtk_status_icon_set_visible(applet->m_pStatusIcon,false);
+        pid_t pid = vfork();
+        if (pid < 0)
+            std::cerr << "vfork failed\n";
+        if (pid == 0)
+        { /* child */
+            execlp("abrt-gui", "abrt-gui", (char*) NULL);
+            std::cerr << "can't exec abrt-gui\n";
+            exit(1);
+        }
+        gtk_status_icon_set_visible(applet->m_pStatusIcon, false);
     }
 }
 
@@ -165,7 +170,7 @@ void CApplet::Disable(const char *reason)
 
 void CApplet::Enable(const char *reason)
 {
-    /* restore the original icon*/
+    /* restore the original icon */
     m_bDaemonRunning = true;
     SetIconTooltip(reason);
     gtk_status_icon_set_from_stock(m_pStatusIcon,GTK_STOCK_DIALOG_WARNING);
@@ -175,7 +180,7 @@ void CApplet::Enable(const char *reason)
 int CApplet::AddEvent(int pUUID, const std::string& pProgname)
 {
     m_mapEvents[pUUID] = "pProgname";
-    SetIconTooltip("Pending events: %i",m_mapEvents.size());
+    SetIconTooltip("Pending events: %i", m_mapEvents.size());
     return 0;
 }
 
diff --git a/src/Applet/CCApplet.h b/src/Applet/CCApplet.h
index b2ce0a2..ff65b03 100644
--- a/src/Applet/CCApplet.h
+++ b/src/Applet/CCApplet.h
@@ -1,22 +1,22 @@
-/* 
-    Copyright (C) 2009  Jiri Moskovcak (jmoskovc@redhat.com) 
-    Copyright (C) 2009  RedHat inc. 
- 
-    This program is free software; you can redistribute it and/or modify 
-    it under the terms of the GNU General Public License as published by 
-    the Free Software Foundation; either version 2 of the License, or 
-    (at your option) any later version. 
- 
-    This program is distributed in the hope that it will be useful, 
-    but WITHOUT ANY WARRANTY; without even the implied warranty of 
-    MERCHANTABILITY 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, write to the Free Software Foundation, Inc., 
-    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. 
+/*
+    Copyright (C) 2009  Jiri Moskovcak (jmoskovc@redhat.com)
+    Copyright (C) 2009  RedHat inc.
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY 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, write to the Free Software Foundation, Inc.,
+    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
     */
-    
+
 #ifndef CC_APPLET_H_
 #define CC_APPLET_H_
 
@@ -26,10 +26,6 @@
 #include <DBusClientProxy.h>
 #include<libnotify/notify.h>
 
-#ifndef BIN_DIR
-    #define BIN_DIR "/usr/bin"
-#endif
-
 class CApplet
 : public CDBusClient_proxy,
   public DBus::IntrospectableProxy,
@@ -41,7 +37,7 @@ class CApplet
         std::map<int, std::string > m_mapEvents;
         DaemonWatcher *m_pDaemonWatcher;
         bool m_bDaemonRunning;
-	public:
+    public:
         CApplet(DBus::Connection &connection, const char *path, const char *name);
         ~CApplet();
         void ShowIcon();
@@ -59,7 +55,7 @@ class CApplet
         int RemoveEvent(int pUUID);
         void ConnectCrashHandler(void (*pCrashHandler)(const char *progname));
         static void DaemonStateChange_cb(bool running, void* data);
-        
+
     protected:
         //@@TODO applet menus
         static void OnAppletActivate_CB(GtkStatusIcon *status_icon,gpointer user_data);
@@ -68,11 +64,11 @@ class CApplet
                             guint          activate_time,
                             gpointer       user_data);
     private:
-    /* dbus stuff */
+        /* dbus stuff */
         void Crash(std::string &value);
 
-    /* the real signal handler called to handle the signal */
-    void (*m_pCrashHandler)(const char *progname);
+        /* the real signal handler called to handle the signal */
+        void (*m_pCrashHandler)(const char *progname);
 };
 
 #endif /*CC_APPLET_H_*/
diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp
index fd69145..13e12d0 100644
--- a/src/Daemon/CrashWatcher.cpp
+++ b/src/Daemon/CrashWatcher.cpp
@@ -684,6 +684,7 @@ static gboolean waitsignal_dispatch(GSource *source, GSourceFunc callback, gpoin
 {
     SignalSource *ssrc = (SignalSource*) source;
     ssrc->watcher->StopRun();
+    return 1;
 }
 
 /* daemon loop with glib */