DBus: Many fixes to client -> cli works again, changed JobDone notification

12 files changed, 124 insertions, 56 deletions

diff --git a/lib/CommLayer/CommLayerClientDBus.cpp b/lib/CommLayer/CommLayerClientDBus.cpp
index 6f4e4b51..77983463 100644
--- a/lib/CommLayer/CommLayerClientDBus.cpp
+++ b/lib/CommLayer/CommLayerClientDBus.cpp
@@ -1,6 +1,7 @@
 #include "CommLayerClientDBus.h"
 CCommLayerClientDBus::CCommLayerClientDBus(DBus::Connection &connection, const char *path, const char *name)
-: DBus::ObjectProxy(connection, path, name)
+: CDBusClient_proxy(connection),
+  DBus::ObjectProxy(connection, path, name)
 {
 }
 CCommLayerClientDBus::~CCommLayerClientDBus()
diff --git a/lib/CommLayer/CommLayerServer.h b/lib/CommLayer/CommLayerServer.h
index b9174d1a..b67b5966 100644
--- a/lib/CommLayer/CommLayerServer.h
+++ b/lib/CommLayer/CommLayerServer.h
@@ -47,7 +47,7 @@ class CCommLayerServer{
         virtual void Error(const std::string& arg1) {}
         virtual void Update(const std::string& pDest, const std::string& pMessage) {};
         virtual void Warning(const std::string& pDest, const std::string& pMessage) {};
-        virtual void JobDone(uint64_t pJobID) {};
+        virtual void JobDone(const std::string &pDest, uint64_t pJobID) {};
 };
 
 #endif //COMMLAYERSERVER_H_
diff --git a/lib/CommLayer/CommLayerServerDBus.cpp b/lib/CommLayer/CommLayerServerDBus.cpp
index d3e00c1f..ad3c2db7 100644
--- a/lib/CommLayer/CommLayerServerDBus.cpp
+++ b/lib/CommLayer/CommLayerServerDBus.cpp
@@ -53,7 +53,7 @@ uint64_t CCommLayerServerDBus::CreateReport_t(const std::string &pUUID,const std
 {
     unsigned long unix_uid = m_pConn->sender_unix_uid(pSender.c_str());
     map_crash_report_t crashReport;
-    uint64_t job_id = m_pObserver->CreateReport_t(pUUID, to_string(unix_uid));
+    uint64_t job_id = m_pObserver->CreateReport_t(pUUID, to_string(unix_uid), pSender);
     return job_id;
 }
 
@@ -99,7 +99,7 @@ void CCommLayerServerDBus::Update(const std::string& pDest, const std::string& p
     CDBusServer_adaptor::Update(pDest, pMessage);
 }
 
-void CCommLayerServerDBus::JobDone(uint64_t pJobID)
+void CCommLayerServerDBus::JobDone(const std::string &pDest, uint64_t pJobID)
 {
-    CDBusServer_adaptor::JobDone(pJobID);
+    CDBusServer_adaptor::JobDone(pDest, pJobID);
 }
diff --git a/lib/CommLayer/CommLayerServerDBus.h b/lib/CommLayer/CommLayerServerDBus.h
index 17b7fdc6..df5d93d5 100644
--- a/lib/CommLayer/CommLayerServerDBus.h
+++ b/lib/CommLayer/CommLayerServerDBus.h
@@ -31,6 +31,6 @@ class CCommLayerServerDBus
         virtual void AnalyzeComplete(map_crash_report_t arg1);
         virtual void Error(const std::string& arg1);
         virtual void Update(const std::string& pDest, const std::string& pMessage);
-        virtual void JobDone(uint64_t pJobID);
+        virtual void JobDone(const std::string &pDest, uint64_t pJobID);
 };
 
diff --git a/lib/CommLayer/DBusClientProxy.h b/lib/CommLayer/DBusClientProxy.h
index 4065307b..52ba75d5 100644
--- a/lib/CommLayer/DBusClientProxy.h
+++ b/lib/CommLayer/DBusClientProxy.h
@@ -112,13 +112,30 @@ public:
 class CDBusClient_proxy
  : public DBus::InterfaceProxy
 {
+private:
+    bool m_bJobDone;
+    uint64_t m_iPendingJobID;
+    GMainLoop *gloop;
+    std::string m_sConnName;
 public:
 
+    
     CDBusClient_proxy()
     : DBus::InterfaceProxy(CC_DBUS_IFACE)
     {
+        connect_signal(CDBusClient_proxy, Crash, _Crash_stub);
+        connect_signal(CDBusClient_proxy, JobDone, _JobDone_stub);
+        m_sConnName = "";
+    }
+    
+    CDBusClient_proxy(::DBus::Connection &pConnection)
+    : DBus::InterfaceProxy(CC_DBUS_IFACE)
+    {
+        gloop = g_main_loop_new(NULL, false);
         //# define connect_signal(interface, signal, callback)
         connect_signal(CDBusClient_proxy, Crash, _Crash_stub);
+        connect_signal(CDBusClient_proxy, JobDone, _JobDone_stub);
+        m_sConnName = pConnection.unique_name();
     }
 
 public:
@@ -168,6 +185,7 @@ public:
     
     map_crash_report_t CreateReport(const std::string& pUUID)
     {
+        m_bJobDone = false;
         DBus::CallMessage call;
         
         DBus::MessageIter wi = call.writer();
@@ -176,10 +194,10 @@ public:
         call.member("CreateReport");
         DBus::Message ret = invoke_method(call);
         DBus::MessageIter ri = ret.reader();
-
-        map_crash_report_t argout;
-        ri >> argout;
-        return argout;
+        ri >> m_iPendingJobID;
+        //FIXME: what if the report is created before we start the loop? (we miss the signal and get stuck in the loop)
+        g_main_loop_run(gloop);
+        return GetJobResult(m_iPendingJobID);
     };
     
     void Report(map_crash_report_t pReport)
@@ -194,12 +212,27 @@ public:
         DBus::MessageIter ri = ret.reader();
     }
     
+    map_crash_report_t GetJobResult(uint64_t pJobID)
+    {
+        DBus::CallMessage call;
+        
+        DBus::MessageIter wi = call.writer();
+
+        wi << pJobID;
+        call.member("GetJobResult");
+        DBus::Message ret = invoke_method(call);
+        DBus::MessageIter ri = ret.reader();
+        map_crash_report_t argout;
+        ri >> argout;
+        return argout;
+    }
+    
 public:
 
     /* signal handlers for this interface
      */
-     virtual void Crash(std::string& value) = 0;
-
+    virtual void Crash(std::string& value){}
+    
 private:
 
     /* unmarshalers (to unpack the DBus message before calling the actual signal handler)
@@ -211,4 +244,16 @@ private:
         std::string value; ri >> value;
         Crash(value);
     }
+    
+    void _JobDone_stub(const ::DBus::SignalMessage &sig)
+    {
+        DBus::MessageIter ri = sig.reader();
+        std::string dest;
+        ri >> dest;
+        if(m_sConnName == dest)
+        {
+            ri >> m_iPendingJobID;
+            g_main_loop_quit(gloop);
+        }
+    }
 };
diff --git a/lib/CommLayer/DBusServerProxy.h b/lib/CommLayer/DBusServerProxy.h
index b55172be..1b8159b6 100644
--- a/lib/CommLayer/DBusServerProxy.h
+++ b/lib/CommLayer/DBusServerProxy.h
@@ -118,10 +118,11 @@ public:
     }
     
     
-    void JobDone(uint64_t job_id)
+    void JobDone(const std::string &pDest, uint64_t job_id)
     {
         ::DBus::SignalMessage sig("JobDone");
         ::DBus::MessageIter wi = sig.writer();
+        wi << pDest;
         wi << job_id;
         emit_signal(sig);
     }
diff --git a/lib/CommLayer/Observer.h b/lib/CommLayer/Observer.h
index 9b476f1f..880a7923 100644
--- a/lib/CommLayer/Observer.h
+++ b/lib/CommLayer/Observer.h
@@ -9,13 +9,13 @@ class CObserver {
     public:
         //CObserver();
         virtual ~CObserver() {}
-        virtual void Status(const std::string& pMessage) = 0;
-        virtual void Debug(const std::string& pMessage) = 0;
-        virtual void Warning(const std::string& pMessage) = 0;
+        virtual void Status(const std::string& pMessage, const std::string& pDest="0") = 0;
+        virtual void Debug(const std::string& pMessage, const std::string& pDest="0") = 0;
+        virtual void Warning(const std::string& pMessage, const std::string& pDest="0") = 0;
 /* this should be implemented in daemon */
         virtual vector_crash_infos_t GetCrashInfos(const std::string &pSender) = 0;
         virtual map_crash_report_t CreateReport(const std::string &pUUID,const std::string &pSender) = 0;
-        virtual uint64_t CreateReport_t(const std::string &pUUID,const std::string &pSender){std::cout << "DEFAULT OBSERVER";return 0;};
+        virtual uint64_t CreateReport_t(const std::string &pUUID,const std::string &pUID, const std::string &pSender){std::cout << "DEFAULT OBSERVER";return 0;};
         virtual bool Report(map_crash_report_t pReport, const std::string &pSender) = 0;
         virtual bool DeleteDebugDump(const std::string& pUUID, const std::string& pSender) = 0;
         virtual map_crash_report_t GetJobResult(uint64_t pJobID, const std::string &pSender) = 0;
diff --git a/src/Applet/CCApplet.h b/src/Applet/CCApplet.h
index 914ddc5e..b2ce0a2a 100644
--- a/src/Applet/CCApplet.h
+++ b/src/Applet/CCApplet.h
@@ -69,8 +69,8 @@ class CApplet
                             gpointer       user_data);
     private:
     /* dbus stuff */
-            void Crash(std::string &value);
-    private:
+        void Crash(std::string &value);
+
     /* the real signal handler called to handle the signal */
     void (*m_pCrashHandler)(const char *progname);
 };
diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp
index 0b521c5e..fd691452 100644
--- a/src/Daemon/CrashWatcher.cpp
+++ b/src/Daemon/CrashWatcher.cpp
@@ -169,9 +169,9 @@ void *CCrashWatcher::create_report(void *arg){
         }
         /* only one thread can write */
         pthread_mutex_lock(&(thread_data->daemon->m_pJobsMutex));
-        thread_data->daemon->pending_jobs[thread_data->thread_id] = crashReport;
+        thread_data->daemon->pending_jobs[std::string(thread_data->UID)][thread_data->thread_id] = crashReport;
         pthread_mutex_unlock(&(thread_data->daemon->m_pJobsMutex));
-        thread_data->daemon->m_pCommLayer->JobDone(thread_data->thread_id);
+        thread_data->daemon->m_pCommLayer->JobDone(thread_data->dest, thread_data->thread_id);
     }
     catch (CABRTException& e)
     {
@@ -180,6 +180,7 @@ void *CCrashWatcher::create_report(void *arg){
             /* free strduped strings */
             free(thread_data->UUID);
             free(thread_data->UID);
+            free(thread_data->dest);
             free(thread_data);
             throw e;
         }
@@ -188,6 +189,7 @@ void *CCrashWatcher::create_report(void *arg){
     /* free strduped strings */
     free(thread_data->UUID);
     free(thread_data->UID);
+    free(thread_data->dest);
     free(thread_data);
 }
 
@@ -375,22 +377,22 @@ void CCrashWatcher::SetUpCron()
     }
 }
 
-void CCrashWatcher::Status(const std::string& pMessage)
+void CCrashWatcher::Status(const std::string& pMessage, const std::string& pDest)
 {
     std::cout << "Update: " + pMessage << std::endl;
     //FIXME: send updates only to job owner
-    if (m_pCommLayer != NULL)
-        m_pCommLayer->Update("0",pMessage);
+    if(m_pCommLayer != NULL)
+       m_pCommLayer->Update(pDest,pMessage);
 }
 
-void CCrashWatcher::Warning(const std::string& pMessage)
+void CCrashWatcher::Warning(const std::string& pMessage, const std::string& pDest)
 {
     std::cerr << "Warning: " + pMessage << std::endl;
-    if (m_pCommLayer != NULL)
-        m_pCommLayer->Warning("0",pMessage);
+    if(m_pCommLayer != NULL)
+       m_pCommLayer->Warning(pDest,pMessage);
 }
 
-void CCrashWatcher::Debug(const std::string& pMessage)
+void CCrashWatcher::Debug(const std::string& pMessage, const std::string& pDest)
 {
     //some logic to add logging levels?
     std::cout << "Debug: " + pMessage << std::endl;
@@ -771,17 +773,26 @@ vector_crash_infos_t CCrashWatcher::GetCrashInfos(const std::string &pUID)
     return retval;
 }
 
-uint64_t CCrashWatcher::CreateReport_t(const std::string &pUUID,const std::string &pUID)
+uint64_t CCrashWatcher::CreateReport_t(const std::string &pUUID,const std::string &pUID, const std::string &pSender)
 {
     thread_data_t *thread_data = (thread_data_t *)xzalloc(sizeof(thread_data_t));
-    thread_data->UUID = xstrdup(pUUID.c_str());
-    thread_data->UID = xstrdup(pUID.c_str());
-    thread_data->daemon = this;
-    if (pthread_create(&(thread_data->thread_id), NULL, create_report, (void *)thread_data) != 0)
+    if (thread_data != NULL)
     {
-        throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CreateReport_t(): Cannot create thread!");
+        thread_data->UUID = xstrdup(pUUID.c_str());
+        thread_data->UID = xstrdup(pUID.c_str());
+        thread_data->dest = xstrdup(pSender.c_str());
+        thread_data->daemon = this;
+        if(pthread_create(&(thread_data->thread_id), NULL, create_report, (void *)thread_data) != 0)
+        {
+            throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CreateReport_t(): Cannot create thread!");
+        }
+    }
+    else
+    {
+        throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CreateReport_t(): Cannot allocate memory!");
     }
-    return (uint64_t) thread_data->thread_id;
+    //FIXME: we don't use this value anymore, so fix the API
+    return 0;
 }
 
 bool CCrashWatcher::Report(map_crash_report_t pReport, const std::string& pUID)
@@ -840,7 +851,11 @@ bool CCrashWatcher::DeleteDebugDump(const std::string& pUUID, const std::string&
     return true;
 }
 
-map_crash_report_t CCrashWatcher::GetJobResult(uint64_t pJobID, const std::string& pDBusSender)
+map_crash_report_t CCrashWatcher::GetJobResult(uint64_t pJobID, const std::string& pSender)
 {
-    return pending_jobs[pJobID];
+    /* FIXME: once we return the result, we should remove it from map to free memory
+       - use some TTL to clean the memory even if client won't get it
+       - if we don't find it in the cache we should try to ask MW to get it again??
+    */
+    return pending_jobs[pSender][pJobID];
 }
diff --git a/src/Daemon/CrashWatcher.h b/src/Daemon/CrashWatcher.h
index 5a095b17..d8fc9aa8 100644
--- a/src/Daemon/CrashWatcher.h
+++ b/src/Daemon/CrashWatcher.h
@@ -74,10 +74,18 @@ class CCrashWatcher
            pthread_t  thread_id;
            char* UUID;
            char* UID;
+           char *dest;
            CCrashWatcher *daemon;
         } thread_data_t;
         
-        std::map <int, map_crash_report_t> pending_jobs;
+        /**
+         * Map to cache the results from CreateReport_t
+         * <UID, <UUID, result>>
+         */
+         std::map <const std::string, std::map <int, map_crash_report_t > > pending_jobs;
+        /**
+        * mutex to protect pending_jobs from being accesed by multiple threads at the same time
+        */
         pthread_mutex_t m_pJobsMutex;
 
         static gboolean handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointer data);
@@ -120,15 +128,15 @@ class CCrashWatcher
         virtual vector_crash_infos_t GetCrashInfos(const std::string &pUID);
         /*FIXME: fix CLI and remove this stub*/
         virtual map_crash_report_t CreateReport(const std::string &pUUID,const std::string &pUID){map_crash_report_t retval; return retval;};
-        uint64_t CreateReport_t(const std::string &pUUID,const std::string &pUID);
+        uint64_t CreateReport_t(const std::string &pUUID,const std::string &pUID, const std::string &pSender);
         virtual bool Report(map_crash_report_t pReport, const std::string &pUID);
         virtual bool DeleteDebugDump(const std::string& pUUID, const std::string& pUID);
-        virtual map_crash_report_t GetJobResult(uint64_t pJobID, const std::string& pDBusSender);
+        virtual map_crash_report_t GetJobResult(uint64_t pJobID, const std::string& pSender);
 
         /* Observer methods */
-        void Status(const std::string& pMessage);
-        void Debug(const std::string& pMessage);
-        void Warning(const std::string& pMessage);
+        void Status(const std::string& pMessage,const std::string& pDest="0");
+        void Debug(const std::string& pMessage, const std::string& pDest="0");
+        void Warning(const std::string& pMessage, const std::string& pDest="0");
 };
 
 #endif /*CRASHWATCHER_H_*/
diff --git a/src/Gui/CCDBusBackend.py b/src/Gui/CCDBusBackend.py
index cac0ec52..546e1070 100644
--- a/src/Gui/CCDBusBackend.py
+++ b/src/Gui/CCDBusBackend.py
@@ -20,7 +20,6 @@ class DBusManager(gobject.GObject):
     # and later with policyKit
     bus = None
     uniq_name = None
-    pending_jobs = []
     def __init__(self):
         session = None
         # binds the dbus to glib mainloop
@@ -128,7 +127,7 @@ class DBusManager(gobject.GObject):
         if not self.bus:
             self.bus = dbus.SystemBus()
             self.bus.add_signal_receiver(self.owner_changed_cb,"NameOwnerChanged", dbus_interface="org.freedesktop.DBus")
-        #self.uniq_name = bus.get_unique_name()
+        self.uniq_name = self.bus.get_unique_name()
         if not self.bus:
             raise Exception("Can't connect to dbus")
         if self.bus.name_has_owner(CC_NAME):
@@ -153,17 +152,16 @@ class DBusManager(gobject.GObject):
             raise Exception("Please check if abrt daemon is running.")
 
     def addJob(self, job_id):
-        self.pending_jobs.append(job_id)
+        pass
+        #self.pending_jobs.append(job_id)
         
-    def jobdone_cb(self, job_id):
-        #if self.uniq_name == client_id:
-        try:
-            self.pending_jobs.index(job_id)
-        except:
-            return
-        dump = self.cc.GetJobResult(job_id)
-        if dump:
-            self.emit("analyze-complete", dump)
+    def jobdone_cb(self, dest, job_id):
+        if self.uniq_name == dest:
+            dump = self.cc.GetJobResult(job_id)
+            if dump:
+                self.emit("analyze-complete", dump)
+            else:
+                raise Exception("Daemon did't return valid report info")
         
     def getReport(self, UUID):
         try:
diff --git a/src/Gui/CCMainWindow.py b/src/Gui/CCMainWindow.py
index 9d9ff4d9..c2f75ab2 100644
--- a/src/Gui/CCMainWindow.py
+++ b/src/Gui/CCMainWindow.py
@@ -145,7 +145,7 @@ class MainWindow():
         try:
             dumplist = getDumpList(self.ccdaemon, refresh=True)
         except Exception, e:
-            gui_error_message("Error while loading the dumplist, please check if abrt daemon is running\n %s" % e.message)
+            gui_error_message("Error while loading the dumplist, please check if abrt daemon is running\n %s" % e)
         for entry in dumplist:
             try:
                 icon = get_icon_for_package(self.theme, entry.getPackageName())