From ee2e495f2eafaa8a2f65ad769bef6761e7f02856 Mon Sep 17 00:00:00 2001 From: Jiri Moskovcak Date: Mon, 3 Aug 2009 12:31:57 +0200 Subject: DBus: Many fixes to client -> cli works again, changed JobDone notification --- lib/CommLayer/CommLayerClientDBus.cpp | 3 +- lib/CommLayer/CommLayerServer.h | 2 +- lib/CommLayer/CommLayerServerDBus.cpp | 6 ++-- lib/CommLayer/CommLayerServerDBus.h | 2 +- lib/CommLayer/DBusClientProxy.h | 57 +++++++++++++++++++++++++++++++---- lib/CommLayer/DBusServerProxy.h | 3 +- lib/CommLayer/Observer.h | 8 ++--- src/Applet/CCApplet.h | 4 +-- src/Daemon/CrashWatcher.cpp | 51 ++++++++++++++++++++----------- src/Daemon/CrashWatcher.h | 20 ++++++++---- src/Gui/CCDBusBackend.py | 22 ++++++-------- src/Gui/CCMainWindow.py | 2 +- 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 pending_jobs; + /** + * Map to cache the results from CreateReport_t + * > + */ + std::map > 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()) -- cgit