From 7a3a5ce3af5ea92835f6b65bad4f347e844a3408 Mon Sep 17 00:00:00 2001 From: Ron Pedde Date: Wed, 12 Jul 2006 04:10:21 +0000 Subject: [PATCH] Fix memory leak in scan-xml, closing #173 --- src/Makefile.am | 2 +- src/db-generic.c | 5 +++++ src/db-sql-sqlite2.c | 4 ++++ src/db-sql.c | 8 ++++---- src/err.c | 30 ++++++++++++++++++++++++++---- src/err.h | 1 + src/main.c | 21 +++------------------ src/plugin.c | 8 ++------ src/scan-xml.c | 16 ++++++++++------ 9 files changed, 56 insertions(+), 39 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index a05456c3..9bba37a7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -56,7 +56,7 @@ mt_daapd_SOURCES = main.c daapd.h rend.h uici.c uici.h webserver.c \ rxml.c rxml.h redblack.c redblack.h scan-mp3.c scan-mp4.c scan-aif.c \ scan-xml.c scan-wma.c scan-aac.c scan-aac.h scan-wav.c scan-url.c \ smart-parser.c smart-parser.h xml-rpc.c xml-rpc.h \ - os.h ll.c ll.h conf.c conf.h compat.c compat.h \ + os.h ll.c ll.h conf.c conf.h compat.c compat.h util.c util.h \ os-unix.h os-unix.c os.h plugin.c plugin.h db-sql-updates.c \ $(PRENDSRC) $(ORENDSRC) $(HRENDSRC) $(OGGVORBISSRC) $(FLACSRC) \ $(MUSEPACKSRC) $(SQLITEDB) $(SQLITE3DB) $(SQLDB) $(GDBM) diff --git a/src/db-generic.c b/src/db-generic.c index 5be6fb38..32cac22f 100644 --- a/src/db-generic.c +++ b/src/db-generic.c @@ -435,9 +435,11 @@ int db_wantsmeta(MetaField_t meta, MetaFieldName_t fieldNo) { void db_readlock(void) { int err; + DPRINTF(E_SPAM,L_LOCK,"entering db_readlock\n"); if((err=pthread_rwlock_rdlock(&db_rwlock))) { DPRINTF(E_FATAL,L_DB,"cannot lock rdlock: %s\n",strerror(err)); } + DPRINTF(E_SPAM,L_LOCK,"db_readlock acquired\n"); } /* @@ -448,9 +450,11 @@ void db_readlock(void) { void db_writelock(void) { int err; + // DPRINTF(E_SPAM,L_LOCK,"entering db_writelock\n"); if((err=pthread_rwlock_wrlock(&db_rwlock))) { DPRINTF(E_FATAL,L_DB,"cannot lock rwlock: %s\n",strerror(err)); } + // DPRINTF(E_SPAM,L_LOCK,"db_writelock acquired\n"); } /* @@ -459,6 +463,7 @@ void db_writelock(void) { * useless, but symmetrical */ int db_unlock(void) { + // DPRINTF(E_SPAM,L_LOCK,"releasing db lock\n"); return pthread_rwlock_unlock(&db_rwlock); } diff --git a/src/db-sql-sqlite2.c b/src/db-sql-sqlite2.c index 4147b9bb..531cc7f2 100644 --- a/src/db-sql-sqlite2.c +++ b/src/db-sql-sqlite2.c @@ -81,9 +81,11 @@ int db_sqlite2_enum_begin_helper(char **pe); void db_sqlite2_lock(void) { int err; + // DPRINTF(E_SPAM,L_LOCK,"entering db_sqlite2_lock\n"); if((err=pthread_mutex_lock(&db_sqlite2_mutex))) { DPRINTF(E_FATAL,L_DB,"cannot lock sqlite lock: %s\n",strerror(err)); } + // DPRINTF(E_SPAM,L_LOCK,"acquired db_sqlite2_lock\n"); } /** @@ -92,9 +94,11 @@ void db_sqlite2_lock(void) { void db_sqlite2_unlock(void) { int err; + // DPRINTF(E_SPAM,L_LOCK,"releasing db_sqlite2_lock\n"); if((err=pthread_mutex_unlock(&db_sqlite2_mutex))) { DPRINTF(E_FATAL,L_DB,"cannot unlock sqlite2 lock: %s\n",strerror(err)); } + // DPRINTF(E_SPAM,L_LOCK,"released db_sqlite2_lock\n"); } /** diff --git a/src/db-sql.c b/src/db-sql.c index a9dc586b..134b27aa 100644 --- a/src/db-sql.c +++ b/src/db-sql.c @@ -168,7 +168,6 @@ int db_sql_fetch_row(char **pe, SQL_ROW *row, char *fmt, ...) { va_list ap; - db_sql_need_dispose = 0; *row=NULL; va_start(ap,fmt); @@ -179,20 +178,21 @@ int db_sql_fetch_row(char **pe, SQL_ROW *row, char *fmt, ...) { db_sql_vmfree_fn(query); if(err != DB_E_SUCCESS) { - DPRINTF(E_SPAM,L_DB,"Error: enum_begin failed (error %d): %s\n", + DPRINTF(E_LOG,L_DB,"Error: enum_begin failed (error %d): %s\n", err,(pe) ? *pe : "?"); return err; } err=db_sql_enum_fetch_fn(pe, row); if(err != DB_E_SUCCESS) { - DPRINTF(E_SPAM,L_DB,"Error: enum_fetch failed (error %d): %s\n", + DPRINTF(E_LOG,L_DB,"Error: enum_fetch failed (error %d): %s\n", err,(pe) ? *pe : "?"); db_sql_enum_end_fn(NULL); return err; } if(!(*row)) { + db_sql_need_dispose=0; db_sql_enum_end_fn(NULL); db_get_error(pe,DB_E_NOROWS); return DB_E_NOROWS; @@ -249,8 +249,8 @@ int db_sql_dispose_row(void) { /* don't really need the row */ if(db_sql_need_dispose) { - err=db_sql_enum_end_fn(NULL); db_sql_need_dispose=0; + err=db_sql_enum_end_fn(NULL); } return err; diff --git a/src/err.c b/src/err.c index 98239f58..3fef7a1a 100644 --- a/src/err.c +++ b/src/err.c @@ -47,6 +47,8 @@ # include "plugin.h" #endif +#include "util.h" + #ifndef PACKAGE # define PACKAGE "unknown daemon" #endif @@ -63,7 +65,7 @@ static int err_syslog_open = 0; /** text list of modules to match for setting debug mask */ static char *err_categorylist[] = { "config","webserver","database","scan","query","index","browse", - "playlist","art","daap","main","rend","xml","parse","plugin",NULL + "playlist","art","daap","main","rend","xml","parse","plugin","lock",NULL }; /* @@ -72,8 +74,28 @@ static char *err_categorylist[] = { static int _err_lock(void); static int _err_unlock(void); +static uint32_t _err_get_threadid(void); +/** + * get an integer representation of a thread id + */ +uint32_t _err_get_threadid(void) { + pthread_t tid; + int thread_id; + + memset((void*)&tid,0,sizeof(pthread_t)); + tid = pthread_self(); + + if(sizeof(pthread_t) == sizeof(int)) { + thread_id = (int)tid; + } else { + thread_id = util_djb_hash_block((unsigned char *)&tid,sizeof(pthread_t)); + } + + return thread_id; +} + /** * if we are logging to a file, then re-open the file. This * would help for log rotation @@ -142,7 +164,7 @@ void err_log(int level, unsigned int cat, char *fmt, ...) snprintf(timebuf,sizeof(timebuf),"%04d-%02d-%02d %02d:%02d:%02d", tm_now.tm_year + 1900, tm_now.tm_mon + 1, tm_now.tm_mday, tm_now.tm_hour, tm_now.tm_min, tm_now.tm_sec); - fprintf(err_file,"%s: %s",timebuf,errbuf); + fprintf(err_file,"%s (%08x): %s",timebuf,_err_get_threadid(),errbuf); if(!level) fprintf(err_file,"%s: Aborting\n",timebuf); fflush(err_file); } @@ -316,19 +338,19 @@ extern int err_setdebugmask(char *list) { } if(!err_categorylist[index]) { + _err_unlock(); DPRINTF(E_LOG,L_MISC,"Unknown module: %s\n",token); free(tmpstr); return 1; } else { - DPRINTF(E_DBG,L_MISC,"Adding module %s to debug list (0x%08x)\n",token,rack); err_debugmask |= rack; } } else break; /* !token */ } + _err_unlock(); DPRINTF(E_INF,L_MISC,"Debug mask is 0x%08x\n",err_debugmask); free(tmpstr); - _err_unlock(); return 0; } diff --git a/src/err.h b/src/err.h index 6e8b7816..e8153ebe 100644 --- a/src/err.h +++ b/src/err.h @@ -57,6 +57,7 @@ #define L_XML 0x00001000 /**< xml - xml-rpc.c */ #define L_PARSE 0x00002000 /**< smart playlist parser */ #define L_PLUG 0x00004000 /**< plugins */ +#define L_LOCK 0x00008000 /**< semaphore locks */ #define L_MISC 0x80000000 /**< anything else */ #ifndef TRUE diff --git a/src/main.c b/src/main.c index b6292cf2..30e1bda4 100644 --- a/src/main.c +++ b/src/main.c @@ -81,6 +81,7 @@ #include "db-generic.h" #include "os.h" #include "plugin.h" +#include "util.h" #ifdef HAVE_GETOPT_H # include "getopt.h" @@ -112,22 +113,6 @@ static int main_auth(WS_CONNINFO *pwsc, char *username, char *password); static void txt_add(char *txtrecord, char *fmt, ...); -/** - * simple hash generator - * - * @param str string to hash - * @returns hash - */ -unsigned long djb_hash(char *str) { - unsigned long hash = 5381; - int c; - unsigned char *pstr = (unsigned char *)str; - - while((c = *pstr++)) - hash = ((hash << 5) + hash) + c; - return hash; -} - /** * build a dns text string * @@ -465,8 +450,8 @@ int main(int argc, char *argv[]) { memset(txtrecord,0,sizeof(txtrecord)); txt_add(txtrecord,"txtvers=1"); - txt_add(txtrecord,"Database ID=%0X",djb_hash(servername)); - txt_add(txtrecord,"Machine ID=%0X",djb_hash(servername)); + txt_add(txtrecord,"Database ID=%0X",util_djb_hash_str(servername)); + txt_add(txtrecord,"Machine ID=%0X",util_djb_hash_str(servername)); txt_add(txtrecord,"Machine Name=%s",servername); txt_add(txtrecord,"mtd-version=" VERSION); txt_add(txtrecord,"iTSh Version=131073"); /* iTunes 6.0.4 */ diff --git a/src/plugin.c b/src/plugin.c index 8d79f7f1..3ac1324f 100644 --- a/src/plugin.c +++ b/src/plugin.c @@ -53,11 +53,9 @@ typedef struct tag_pluginentry { } PLUGIN_ENTRY; /* Globals */ -static pthread_key_t _plugin_lock_key; static PLUGIN_ENTRY _plugin_list; static int _plugin_initialized = 0; static char *_plugin_ssc_codecs = NULL; -static pthread_rwlock_t _plugin_lock; static char* _plugin_error_list[] = { "Success.", @@ -128,9 +126,6 @@ PLUGIN_INPUT_FN pi = { * @returns TRUE on success, FALSE otherwise */ int plugin_init(void) { - pthread_rwlock_init(&_plugin_lock,NULL); - pthread_key_create(&_plugin_lock_key, (void*)_plugin_free); - return TRUE; } @@ -718,7 +713,8 @@ int pi_db_enum_start(char **pe, DB_QUERY *pinfo) { pqi->query_type = queryTypeBrowseComposers; } else { if(pe) *pe = strdup("Unsupported browse type"); - sp_dispose(pqi->pt); + if(pqi->pt) + sp_dispose(pqi->pt); pqi->pt = NULL; return -1; /* not really a db error for this */ } diff --git a/src/scan-xml.c b/src/scan-xml.c index 58e3ab6c..a081dbef 100644 --- a/src/scan-xml.c +++ b/src/scan-xml.c @@ -57,7 +57,7 @@ static char *scan_xml_file; /** < The actual file we are scanning */ static struct rbtree *scan_xml_db; #define MAYBECOPY(a) if(mp3.a) pmp3->a = mp3.a -#define MAYBECOPYSTRING(a) if(mp3.a) { free(pmp3->a); pmp3->a = mp3.a; } +#define MAYBECOPYSTRING(a) if(mp3.a) { free(pmp3->a); pmp3->a = mp3.a; mp3.a=NULL; } #define MAYBEFREE(a) if((a)) { free((a)); (a)=NULL; } /** iTunes xml values we are interested in */ @@ -712,9 +712,6 @@ int scan_xml_tracks_section(int action, char *info) { db_add(NULL,pmp3,NULL); db_dispose_item(pmp3); - - memset((void*)&mp3,0,sizeof(MP3FILE)); - MAYBEFREE(song_path); } } else if(is_streaming) { /* add/update a http:// url */ @@ -759,9 +756,16 @@ int scan_xml_tracks_section(int action, char *info) { } db_dispose_item(pmp3); - memset((void*)&mp3,0,sizeof(MP3FILE)); - MAYBEFREE(song_path); } + + /* cleanup what's left */ + MAYBEFREE(mp3.title); + MAYBEFREE(mp3.artist); + MAYBEFREE(mp3.album); + MAYBEFREE(mp3.genre); + MAYBEFREE(mp3.comment); + MAYBEFREE(song_path); + memset((void*)&mp3,0,sizeof(MP3FILE)); } else { return XML_STATE_ERROR; }