The following bug has been CLOSED
======================================================================
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: closed
======================================================================
Date Submitted: 13-Sep-04 23:35 CEST
Last Modified: 13-Dec-04 11:36 CET
======================================================================
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?
----------------------------------------------------------------------
ljackson - 17-Sep-04 22:19 CEST
----------------------------------------------------------------------
Ilja,
I have tested what you stated and you are correct, there seems to be no
need for that code. I did however find some more minor changes I have
attached the diff if you want to check these out, it includes change I
sent to dbmail-dev about the dynamic table prefix defaulting to dbmail_.
thx
----------------------------------------------------------------------
ilja - 20-Sep-04 15:45 CEST
----------------------------------------------------------------------
The fixes are in HEAD.
The fix for the table prefix had to be changed somewhat to really work.
----------------------------------------------------------------------
ilja - 20-Sep-04 15:59 CEST
----------------------------------------------------------------------
fixes are also in 2.0 branch. Only the extra my_free()s and list_freelist()
of course.
----------------------------------------------------------------------
aaron - 15-Oct-04 18:29 CEST
----------------------------------------------------------------------
So did this bug get resolved?
----------------------------------------------------------------------
aaron - 02-Nov-04 04:33 CET
----------------------------------------------------------------------
Ilja, is this bug resolved?
----------------------------------------------------------------------
ilja - 02-Nov-04 09:42 CET
----------------------------------------------------------------------
yep, it's resolved :)
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
17-Sep-04 22:18ljackson File Added: dbmail-2.1cvs091504.mem_nitpicks.diff
17-Sep-04 22:19ljackson Bugnote Added: 0000270
20-Sep-04 15:45ilja Bugnote Added: 0000276
20-Sep-04 15:59ilja Bugnote Added: 0000277
15-Oct-04 18:29aaron Bugnote Added: 0000315
15-Oct-04 23:11michaelg Bug Monitored: michaelg
15-Oct-04 23:11michaelg Bug End Monitor: michaelg
02-Nov-04 04:33aaron Bugnote Added: 0000333
02-Nov-04 09:42ilja Bugnote Added: 0000338
02-Nov-04 09:42ilja Resolution reopened => fixed
02-Nov-04 09:42ilja Status feedback => resolved
13-Dec-04 11:36ilja Status resolved => closed
======================================================================
