From e8030896a5b70576bb4ba2f4d534336ace1cd122 Mon Sep 17 00:00:00 2001 From: Dave West Date: Fri, 7 Dec 2007 15:09:20 +0000 Subject: [PATCH] Hunting a bug in scheduled threads that caused a segflt. Got it and squished it. We needed to clear next and prev before linking the scheduled thread onto the running thread list. Also fixed a memory leak in the rssclient. --- citadel/modules/checkpoint/serv_checkpoint.c | 2 - citadel/modules/expire/serv_expire.c | 2 - citadel/modules/fulltext/serv_fulltext.c | 2 - citadel/modules/network/serv_network.c | 1 + citadel/modules/rssclient/serv_rssclient.c | 2 +- citadel/sysdep.c | 42 +++++++++++++++----- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/citadel/modules/checkpoint/serv_checkpoint.c b/citadel/modules/checkpoint/serv_checkpoint.c index 32bfad00b..1d48ff5e0 100644 --- a/citadel/modules/checkpoint/serv_checkpoint.c +++ b/citadel/modules/checkpoint/serv_checkpoint.c @@ -44,8 +44,6 @@ void *checkpoint_thread(void *arg) { struct CitContext checkpointCC; - CtdlThreadAllocTSD(); - CtdlLogPrintf(CTDL_DEBUG, "checkpoint_thread() initializing\n"); memset(&checkpointCC, 0, sizeof(struct CitContext)); diff --git a/citadel/modules/expire/serv_expire.c b/citadel/modules/expire/serv_expire.c index e807f969b..df65a8dca 100644 --- a/citadel/modules/expire/serv_expire.c +++ b/citadel/modules/expire/serv_expire.c @@ -720,8 +720,6 @@ void *purge_databases(void *args) time_t now; struct tm tm; - CtdlThreadAllocTSD(); - while (!CtdlThreadCheckStop()) { /* Do the auto-purge if the current hour equals the purge hour, * but not if the operation has already been performed in the diff --git a/citadel/modules/fulltext/serv_fulltext.c b/citadel/modules/fulltext/serv_fulltext.c index 7c76d9d64..ef542d829 100644 --- a/citadel/modules/fulltext/serv_fulltext.c +++ b/citadel/modules/fulltext/serv_fulltext.c @@ -339,8 +339,6 @@ void do_fulltext_indexing(void) { void *indexer_thread(void *arg) { struct CitContext indexerCC; - CtdlThreadAllocTSD(); - lprintf(CTDL_DEBUG, "indexer_thread() initializing\n"); memset(&indexerCC, 0, sizeof(struct CitContext)); diff --git a/citadel/modules/network/serv_network.c b/citadel/modules/network/serv_network.c index 5d55b2336..c685ec4d1 100644 --- a/citadel/modules/network/serv_network.c +++ b/citadel/modules/network/serv_network.c @@ -2054,6 +2054,7 @@ void *network_do_queue(void *args) { */ if ( (time(NULL) - last_run) < config.c_net_freq ) { full_processing = 0; + CtdlLogPrintf(CTDL_DEBUG, "Network full processing in %ld seconds.\n", config.c_net_freq - (time(NULL)- last_run)); } /* diff --git a/citadel/modules/rssclient/serv_rssclient.c b/citadel/modules/rssclient/serv_rssclient.c index 4ef2cfb32..e27f888d4 100644 --- a/citadel/modules/rssclient/serv_rssclient.c +++ b/citadel/modules/rssclient/serv_rssclient.c @@ -162,13 +162,13 @@ void rss_save_item(struct rss_item *ri) { CtdlSubmitMsg(msg, recp, NULL); CtdlFreeMessage(msg); - free_recipients(recp); /* write the uidl to the use table so we don't store this item again */ strcpy(ut.ut_msgid, utmsgid); ut.ut_timestamp = time(NULL); cdb_store(CDB_USETABLE, utmsgid, strlen(utmsgid), &ut, sizeof(struct UseTable) ); } + free_recipients(recp); } diff --git a/citadel/sysdep.c b/citadel/sysdep.c index 90fff43e5..67f474052 100644 --- a/citadel/sysdep.c +++ b/citadel/sysdep.c @@ -1519,6 +1519,7 @@ void CtdlThreadGC (void) { struct CtdlThreadNode *this_thread, *that_thread; int workers = 0; + int ret=0; begin_critical_section(S_THREAD_LIST); @@ -1585,11 +1586,17 @@ void CtdlThreadGC (void) /* * Join on the thread to do clean up and prevent memory leaks * Also makes sure the thread has cleaned up after itself before we remove it from the list - * If that thread has no function it must be the garbage collector + * We can join on the garbage collector thread the join should just return EDEADLCK */ - if (that_thread->thread_func) - pthread_join (that_thread->tid, NULL); - + ret = pthread_join (that_thread->tid, NULL); + if (ret == EDEADLK) + CtdlLogPrintf(CTDL_DEBUG, "Garbage collection on own thread.\n"); + else if (ret == EINVAL) + CtdlLogPrintf(CTDL_DEBUG, "Garbage collection, that thread already joined on.\n"); + else if (ret == ESRCH) + CtdlLogPrintf(CTDL_DEBUG, "Garbage collection, no thread to join on.\n"); + else if (ret != 0) + CtdlLogPrintf(CTDL_DEBUG, "Garbage collection, pthread_join returned an unknown error.\n"); /* * Now we own that thread entry */ @@ -1969,6 +1976,10 @@ void ctdl_thread_internal_check_scheduled(void) now = time(NULL); +#ifdef WITH_THREADLOG + CtdlLogPrintf(CTDL_DEBUG, "Checking for scheduled threads to start.\n"); +#endif + this_thread = CtdlThreadSchedList; while(this_thread) { @@ -1978,19 +1989,26 @@ void ctdl_thread_internal_check_scheduled(void) if (now > that_thread->when) { /* Unlink from schedule list */ - if (that_thread->next) - that_thread->next->prev = that_thread->prev; if (that_thread->prev) that_thread->prev->next = that_thread->next; else CtdlThreadSchedList = that_thread->next; - + if (that_thread->next) + that_thread->next->prev = that_thread->prev; + + that_thread->next = that_thread->prev = NULL; +#ifdef WITH_THREADLOG + CtdlLogPrintf(CTDL_DEBUG, "About to start scheduled thread \"%s\".\n", that_thread->name); +#endif begin_critical_section(S_THREAD_LIST); if (CT->state > CTDL_THREAD_STOP_REQ) { /* Only start it if the system is not stopping */ pthread_mutex_lock(&that_thread->ThreadMutex); if (ctdl_thread_internal_start_scheduled (that_thread) == NULL) { +#ifdef WITH_THREADLOG + CtdlLogPrintf(CTDL_DEBUG, "Failed to start scheduled thread \"%s\".\n", that_thread->name); +#endif pthread_mutex_unlock(&that_thread->ThreadMutex); pthread_mutex_destroy(&(that_thread->ThreadMutex)); pthread_cond_destroy(&(that_thread->ThreadCond)); @@ -2001,14 +2019,20 @@ void ctdl_thread_internal_check_scheduled(void) } else { - CtdlLogPrintf(CTDL_INFO, "Thread system, Started a sceduled thread \"%s\".\n", - that_thread->name); + CtdlLogPrintf(CTDL_INFO, "Thread system, Started a scheduled thread \"%s\" (%ld).\n", + that_thread->name, that_thread->tid); pthread_mutex_unlock(&that_thread->ThreadMutex); ctdl_thread_internal_calc_loadavg(); } } end_critical_section(S_THREAD_LIST); } + else + { +#ifdef WITH_THREADLOG + CtdlLogPrintf(CTDL_DEBUG, "Thread \"%s\" will start in %ld seconds.\n", that_thread->name, that_thread->when - time(NULL)); +#endif + } } end_critical_section(S_SCHEDULE_LIST); } -- 2.30.2