From 76585105247365dd9f84997d933eeabd4303aabd Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 7 Sep 2009 18:36:29 +0200 Subject: preparatory work for proper (i.e. unicast) server->client dbus communication + /* send unicast dbus signal */ + if (!dbus_message_set_destination(msg, pDest)) + die_out_of_memory(); Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/Daemon/CrashWatcher.cpp') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 06afabd..1b17af6 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -142,6 +142,7 @@ static void *create_report(void *arg) pthread_mutex_lock(&g_pJobsMutex); g_pending_jobs[std::string(thread_data->UID)][uint64_t(thread_data->thread_id)] = crashReport; pthread_mutex_unlock(&g_pJobsMutex); + g_pCommLayer->JobDone(thread_data->dest, uint64_t(thread_data->thread_id)); } catch (CABRTException& e) @@ -179,7 +180,7 @@ uint64_t CreateReport_t(const char* pUUID, const char* pUID, const char* pSender free(thread_data->dest); free(thread_data); /* The only reason this may happen is system-wide resource starvation, - * or ulimit is exceeded (someoune floods us with CreateReport() dbus calls?) + * or ulimit is exceeded (someone floods us with CreateReport() dbus calls?) */ error_msg("cannot create thread"); return 0; -- cgit From a4257108013f15653724b5ed3b3c208e071567c5 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 8 Sep 2009 13:16:06 +0200 Subject: abrtd: eliminate g_pending_jobs[] map and all corresponding TODOs :) -6k: text data bss dec hexfilename 194741 2656 2384 199781 30c65abrt.t1/abrt-0.0.8.5/src/Daemon/.libs/abrtd 188316 2648 2320 193284 2f304abrt.t2/abrt-0.0.8.5/src/Daemon/.libs/abrtd Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 100 +++++++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 44 deletions(-) (limited to 'src/Daemon/CrashWatcher.cpp') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 1b17af6..1b147ed 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -102,48 +102,60 @@ vector_crash_infos_t GetCrashInfos(const std::string &pUID) return retval; } +/* + * "GetJobResult" is a bit of a misnomer. + * It actually _creates_ a_ report_ and returns the result. + * It is called in two cases: + * (1) by CreateReport dbus call -> CreateReportThread(), in the thread + * (2) by GetJobResult dbus call + * In the second case, it finishes quickly, because previous + * CreateReport dbus call already did all the processing, and we just retrieve + * the result from dump directory, which is fast. + */ +map_crash_report_t GetJobResult(const char* pUUID, const char* pUID) +{ + map_crash_info_t crashReport; + + mw_result_t res = CreateCrashReport(pUUID, pUID, crashReport); + switch (res) + { + case MW_OK: + break; + case MW_IN_DB_ERROR: + warn_client(std::string("Did not find crash with UUID ") + pUUID + " in database"); + break; + case MW_PLUGIN_ERROR: + warn_client("Particular analyzer plugin isn't loaded or there is an error within plugin(s)"); + break; + case MW_CORRUPTED: + case MW_FILE_ERROR: + default: + warn_client(std::string("Corrupted crash with UUID ") + pUUID + ", deleting"); + std::string debugDumpDir = DeleteCrashInfo(pUUID, pUID); + DeleteDebugDumpDir(debugDumpDir); + break; + } + return crashReport; +} + typedef struct thread_data_t { pthread_t thread_id; char* UUID; char* UID; char* dest; } thread_data_t; -static void *create_report(void *arg) +static void* create_report(void* arg) { thread_data_t *thread_data = (thread_data_t *) arg; - map_crash_info_t crashReport; - g_pCommLayer->JobStarted(thread_data->dest, uint64_t(thread_data->thread_id)); + g_pCommLayer->JobStarted(thread_data->dest); - log("Creating report..."); try { - mw_result_t res; - res = CreateCrashReport(thread_data->UUID, thread_data->UID, crashReport); - switch (res) - { - case MW_OK: - break; - case MW_IN_DB_ERROR: - warn_client(std::string("Did not find crash with UUID ") + thread_data->UUID + " in database"); - break; - case MW_PLUGIN_ERROR: - warn_client(std::string("Particular analyzer plugin isn't loaded or there is an error within plugin(s)")); - break; - case MW_CORRUPTED: - case MW_FILE_ERROR: - default: - warn_client(std::string("Corrupted crash with UUID ") + thread_data->UUID + ", deleting"); - std::string debugDumpDir = DeleteCrashInfo(thread_data->UUID, thread_data->UID); - DeleteDebugDumpDir(debugDumpDir); - break; - } - /* only one thread can write */ - pthread_mutex_lock(&g_pJobsMutex); - g_pending_jobs[std::string(thread_data->UID)][uint64_t(thread_data->thread_id)] = crashReport; - pthread_mutex_unlock(&g_pJobsMutex); - - g_pCommLayer->JobDone(thread_data->dest, uint64_t(thread_data->thread_id)); + /* "GetJobResult" is a bit of a misnomer */ + log("Creating report..."); + map_crash_info_t crashReport = GetJobResult(thread_data->UUID, thread_data->UID); + g_pCommLayer->JobDone(thread_data->dest, thread_data->UUID); } catch (CABRTException& e) { @@ -167,13 +179,18 @@ static void *create_report(void *arg) /* Bogus value. pthreads require us to return void* */ return NULL; } -uint64_t CreateReport_t(const char* pUUID, const char* pUID, const char* pSender) +int CreateReportThread(const char* pUUID, const char* pUID, const char* pSender) { thread_data_t *thread_data = (thread_data_t *)xzalloc(sizeof(thread_data_t)); thread_data->UUID = xstrdup(pUUID); thread_data->UID = xstrdup(pUID); thread_data->dest = xstrdup(pSender); - if (pthread_create(&thread_data->thread_id, NULL, create_report, (void *)thread_data) != 0) +//TODO: do we need this? +//pthread_attr_t attr; +//pthread_attr_init(&attr); +//pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + int r = pthread_create(&thread_data->thread_id, NULL, create_report, thread_data); + if (r != 0) { free(thread_data->UUID); free(thread_data->UID); @@ -182,10 +199,14 @@ uint64_t CreateReport_t(const char* pUUID, const char* pUID, const char* pSender /* The only reason this may happen is system-wide resource starvation, * or ulimit is exceeded (someone floods us with CreateReport() dbus calls?) */ - error_msg("cannot create thread"); - return 0; + error_msg("Can't create thread"); + } + else + { + VERB3 log("Thread %llx created", (unsigned long long)thread_data->thread_id); } - return uint64_t(thread_data->thread_id); +//pthread_attr_destroy(&attr); + return r; } bool DeleteDebugDump(const std::string& pUUID, const std::string& pUID) @@ -208,12 +229,3 @@ bool DeleteDebugDump(const std::string& pUUID, const std::string& pUID) } return true; } - -map_crash_report_t GetJobResult(uint64_t pJobID, const std::string& pSender) -{ - /* 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 g_pending_jobs[pSender][pJobID]; -} -- cgit From 8198cd06195f4217fd6b1afb675f3a316c951a1e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 8 Sep 2009 15:43:18 +0200 Subject: style fixes, trivial code changes only Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'src/Daemon/CrashWatcher.cpp') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 1b147ed..5ad419b 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -116,6 +116,11 @@ map_crash_report_t GetJobResult(const char* pUUID, const char* pUID) { map_crash_info_t crashReport; + /* FIXME: starting from here, any shared data must be protected with a mutex. + * For example, CreateCrashReport does: + * g_pPluginManager->GetDatabase(g_settings_sDatabase); + * which is unsafe wrt concurrent updates to g_pPluginManager state. + */ mw_result_t res = CreateCrashReport(pUUID, pUID, crashReport); switch (res) { @@ -152,7 +157,7 @@ static void* create_report(void* arg) try { - /* "GetJobResult" is a bit of a misnomer */ + /* "GetJobResult" is a bit of a misnomer */ log("Creating report..."); map_crash_info_t crashReport = GetJobResult(thread_data->UUID, thread_data->UID); g_pCommLayer->JobDone(thread_data->dest, thread_data->UUID); @@ -213,8 +218,7 @@ bool DeleteDebugDump(const std::string& pUUID, const std::string& pUID) { try { - std::string debugDumpDir; - debugDumpDir = DeleteCrashInfo(pUUID, pUID); + std::string debugDumpDir = DeleteCrashInfo(pUUID, pUID); DeleteDebugDumpDir(debugDumpDir); } catch (CABRTException& e) -- cgit From ff2627e7c597e50025ca4d91ff8168eec80f9054 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Tue, 8 Sep 2009 17:05:57 +0200 Subject: make Warning, Error and Update send unicast dbus messages Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'src/Daemon/CrashWatcher.cpp') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 5ad419b..6c4d654 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -24,19 +24,18 @@ #include "ABRTException.h" #include "CrashWatcher.h" -void CCrashWatcher::Status(const std::string& pMessage, uint64_t pJobID) +void CCrashWatcher::Status(const std::string& pMessage, const char* peer, uint64_t pJobID) { - log("Update: %s", pMessage.c_str()); - //FIXME: send updates only to job owner + VERB1 log("Update('%s'): %s", peer, pMessage.c_str()); if (g_pCommLayer != NULL) - g_pCommLayer->Update(pMessage, pJobID); + g_pCommLayer->Update(pMessage, peer, pJobID); } -void CCrashWatcher::Warning(const std::string& pMessage, uint64_t pJobID) +void CCrashWatcher::Warning(const std::string& pMessage, const char* peer, uint64_t pJobID) { - log("Warning: %s", pMessage.c_str()); + VERB1 log("Warning('%s'): %s", peer, pMessage.c_str()); if (g_pCommLayer != NULL) - g_pCommLayer->Warning(pMessage, pJobID); + g_pCommLayer->Warning(pMessage, peer, pJobID); } CCrashWatcher::CCrashWatcher() @@ -147,38 +146,44 @@ typedef struct thread_data_t { pthread_t thread_id; char* UUID; char* UID; - char* dest; + char* peer; } thread_data_t; static void* create_report(void* arg) { thread_data_t *thread_data = (thread_data_t *) arg; - g_pCommLayer->JobStarted(thread_data->dest); + g_pCommLayer->JobStarted(thread_data->peer); + + /* Client name is per-thread, need to set it */ + set_client_name(thread_data->peer); try { /* "GetJobResult" is a bit of a misnomer */ log("Creating report..."); map_crash_info_t crashReport = GetJobResult(thread_data->UUID, thread_data->UID); - g_pCommLayer->JobDone(thread_data->dest, thread_data->UUID); + g_pCommLayer->JobDone(thread_data->peer, thread_data->UUID); } catch (CABRTException& e) { if (e.type() == EXCEP_FATAL) { + set_client_name(NULL); /* free strduped strings */ free(thread_data->UUID); free(thread_data->UID); - free(thread_data->dest); + free(thread_data->peer); free(thread_data); throw e; } warn_client(e.what()); } + set_client_name(NULL); + /* free strduped strings */ free(thread_data->UUID); free(thread_data->UID); - free(thread_data->dest); + free(thread_data->peer); free(thread_data); /* Bogus value. pthreads require us to return void* */ @@ -189,7 +194,7 @@ int CreateReportThread(const char* pUUID, const char* pUID, const char* pSender) thread_data_t *thread_data = (thread_data_t *)xzalloc(sizeof(thread_data_t)); thread_data->UUID = xstrdup(pUUID); thread_data->UID = xstrdup(pUID); - thread_data->dest = xstrdup(pSender); + thread_data->peer = xstrdup(pSender); //TODO: do we need this? //pthread_attr_t attr; //pthread_attr_init(&attr); @@ -199,7 +204,7 @@ int CreateReportThread(const char* pUUID, const char* pUID, const char* pSender) { free(thread_data->UUID); free(thread_data->UID); - free(thread_data->dest); + free(thread_data->peer); free(thread_data); /* The only reason this may happen is system-wide resource starvation, * or ulimit is exceeded (someone floods us with CreateReport() dbus calls?) -- cgit