A BUGNOTE has been added to this bug. ====================================================================== http://dbmail.org/mantis/bug_view_advanced_page.php?bug_id=0000087 ====================================================================== Reported By: ljackson Assigned To: ilja ====================================================================== Project: DBMail Bug ID: 87 Category: Database layer Reproducibility: always Severity: major Priority: normal Status: feedback ====================================================================== Date Submitted: 13-Sep-04 23:35 CEST Last Modified: 16-Sep-04 11:53 CEST ====================================================================== Summary: Possible memory leak in session and db code for pop3d Description: I have been able to reproduce in both cvs from 080304 and 2.0rc8 an issue where normal connects & lists to the pop3d leaks memory, I have traced it to session code not being freed in the connection cleanup code. ======================================================================
---------------------------------------------------------------------- ilja - 15-Sep-04 12:20 CEST ---------------------------------------------------------------------- I'm taking this patch one step at a time. first: I've added db_session_cleanup() to pop3.c. I've put it in a different place than in the patch. It should clean up the session after errors too. So, it's added after the switch statement in the patch. ---------------------------------------------------------------------- ilja - 15-Sep-04 14:20 CEST ---------------------------------------------------------------------- The auth/authsql.c fix is also in. ---------------------------------------------------------------------- ilja - 15-Sep-04 14:51 CEST ---------------------------------------------------------------------- db.c fixed ---------------------------------------------------------------------- ilja - 15-Sep-04 15:03 CEST ---------------------------------------------------------------------- the fix for mysql/dbmysql.c isn't necessary: mysql_store_result() will return NULL if the query isn't a SELECT query. On top of that mysql_num_fields doesn't work in current versions of MySQL. ---------------------------------------------------------------------- ilja - 15-Sep-04 15:04 CEST ---------------------------------------------------------------------- All relevant fixes are now in dbmail_2_0_branch and HEAD. ---------------------------------------------------------------------- ljackson - 15-Sep-04 19:53 CEST ---------------------------------------------------------------------- Thanks for appling this patch, while I aggree that in Mysql 3.x might not use the mysql_store_result but this works for Mysql 4.x and higher and is recomended from thier documentation. This solves the need for a lot of extra db_free_result's in each of the sql statments that are not selects. Otherwise we still have a memory leak for each on of the statments. Please consider a ifdef or conditional that adds it if your using a Mysql version >= 4.x thanks, leif ---------------------------------------------------------------------- ljackson - 15-Sep-04 19:54 CEST ---------------------------------------------------------------------- also has this patch been merged into 2.1 cvs head? I havn't checked but I belive it is an issue there as well. ---------------------------------------------------------------------- aaron - 15-Sep-04 20:01 CEST ---------------------------------------------------------------------- I thought that DBMail required MySQL 4.0 or higher... if not, it should, IMHO. ---------------------------------------------------------------------- ljackson - 15-Sep-04 20:15 CEST ---------------------------------------------------------------------- In the back of my mind I thought it did as well that is why I didn't feel bad adding that bit of code, *shrug* I thought it made sence. There are still a number of minor memory leaks that are reported from valgrind in thread watching mode if anyone is interested use the latest version of valgrind 2.2.0 and the following cmd line from the compiled directory. valgrind --tool=memcheck --logfile=/tmp/pop3d --leak-check=yes --show-reachable=yes --trace-children=yes -v ./dbmail-pop3d or if you want to really dig on it use valgrind --tool=memcheck --logfile=/tmp/pop3d --leak-check=yes --show-reachable=yes --trace-children=yes --track-fds=yes --time-stamp=yes --num-callers=6 --error-limit=no --show-below-main=yes -v ./dbmail-pop3d it is minor but there are still some cases where memory is being malloced for list node_add and not being freed even on Sig 15. just my 0.02 again. ---------------------------------------------------------------------- ljackson - 15-Sep-04 20:18 CEST ---------------------------------------------------------------------- btw this takes a heafty machine to run the second command, I have a dual ADM 1.4Ghz w/ 6Gb DDR ecc ram that I bang agansit it with and I can upload the ruby test code I use bascialy a little pop and imap script I run multiple times from a perl script. I use my other devel machine a dual ADM 1.4 with 2Gb ram to attack it with. This makes the SQL server I have with Dual Xeon's goto a load of around 5 when running the pop tests and a load of about 12 when running the imap and pop test togther.... evil yes I know. ---------------------------------------------------------------------- ilja - 16-Sep-04 11:53 CEST ---------------------------------------------------------------------- I still don't see why the extra db_free_result() in db_query() is needed. The MySQL docs state this: mysql_store_result() returns a null pointer if the query didn't return a result set (if the query was, for example, an INSERT statement). So.. what's to free if the query was not a SELECT? Bug History Date Modified Username Field Change ====================================================================== 13-Sep-04 23:35ljackson New Bug 13-Sep-04 23:35ljackson File Added: dbmail-2.0rc8.memleakmysql_pop3session.diff 15-Sep-04 12:20ilja Bugnote Added: 0000242 15-Sep-04 12:20ilja Assigned To => ilja 15-Sep-04 12:20ilja Status new => confirmed 15-Sep-04 14:20ilja Bugnote Added: 0000243 15-Sep-04 14:51ilja Bugnote Added: 0000244 15-Sep-04 15:03ilja Bugnote Added: 0000245 15-Sep-04 15:03ilja Status confirmed => assigned 15-Sep-04 15:04ilja Bugnote Added: 0000246 15-Sep-04 15:04ilja Resolution open => fixed 15-Sep-04 15:04ilja Status assigned => resolved 15-Sep-04 19:53ljackson Status resolved => feedback 15-Sep-04 19:53ljackson Bugnote Added: 0000256 15-Sep-04 19:53ljackson Resolution fixed => reopened 15-Sep-04 19:54ljackson Bugnote Added: 0000257 15-Sep-04 20:01aaron Bugnote Added: 0000260 15-Sep-04 20:15ljackson Bugnote Added: 0000262 15-Sep-04 20:18ljackson Bugnote Added: 0000263 16-Sep-04 11:53ilja Bugnote Added: 0000265 ======================================================================
