Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » U++ Library support » U++ SQL » [BUG] Memory leak in MySql package
[BUG] Memory leak in MySql package [message #39611] Fri, 05 April 2013 12:43 Go to next message
Klugier is currently offline  Klugier
Messages: 821
Registered: September 2012
Location: Poland, Kraków
Experienced Contributor
Hello,

I diagnosed serious memory leak in MySql package. According to the following reference http://dev.mysql.com/doc/refman/5.1/en/mysql-library-end.htm l we need to call extra function (mysql_library_end(void)) to make total order.

I enclose patch code (MySql/MySql.cpp - line 132):

void MySqlSession::Close() 
{
	SessionClose();
	if(mysql) {
		mysql_close(mysql);
		mysql_library_end();
		mysql = NULL;
		level = 0;
	}
}


Sincerely,
Klugier


Ultimate++ - one framework to rule them all.

[Updated on: Fri, 05 April 2013 12:44]

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39714 is a reply to message #39611] Fri, 19 April 2013 16:12 Go to previous messageGo to next message
Alboni is currently offline  Alboni
Messages: 203
Registered: January 2012
Location: Deventer, Netherlands
Experienced Member
Hi Klugier,
How did you diagnose the leak?


As I read it:

mysql_library_end() should be called after the last session close. Not after each session close.

I cannot find any mysql_library_init() call anywhere in Upp.
(Yet still it works)

I need to conclude that these calls are made in DllMain() of MySql.dll when loading and unloading the dll.

Doing it explicitly is imho only relevant for programs that statically link MySql (wich Upp does not).
Re: [BUG] Memory leak in MySql package [message #39719 is a reply to message #39714] Sat, 20 April 2013 15:05 Go to previous messageGo to next message
Klugier is currently offline  Klugier
Messages: 821
Registered: September 2012
Location: Poland, Kraków
Experienced Contributor
Hello Alboni,

You are absolutly right, We need to count global number of MySqlSession instances. If the number of instances falls to zero we need to call mysql_library_end().

I use a memory profiler called valgrind. I invoke him with following command:
valgrind --tool=memcheck --leak-check=full --show-reachable=yes ./ProgramName


I enclose completed patch for this issue. I have added new static variable to count number of instances (numberOfInstances) of the class MySqlSession.

Sincerely,
Klugier


Ultimate++ - one framework to rule them all.

[Updated on: Sat, 20 April 2013 15:35]

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39819 is a reply to message #39719] Fri, 03 May 2013 21:48 Go to previous messageGo to next message
BioBytes is currently offline  BioBytes
Messages: 288
Registered: October 2008
Location: France
Experienced Member
Hello Klugier,

Do you know if your MySql patch has been included in the Upp core ?

Regards

Biobytes
Re: [BUG] Memory leak in MySql package [message #39915 is a reply to message #39819] Sat, 11 May 2013 02:31 Go to previous messageGo to next message
Klugier is currently offline  Klugier
Messages: 821
Registered: September 2012
Location: Poland, Kraków
Experienced Contributor
Hello BioBytes,

Now, I am waiting for patch confirmation.

Sincerely,
Klugier


Ultimate++ - one framework to rule them all.
Re: [BUG] Memory leak in MySql package [message #39921 is a reply to message #39719] Sat, 11 May 2013 11:27 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13032
Registered: November 2005
Ultimate Member
klugier wrote on Sat, 20 April 2013 09:05

Hello Alboni,

You are absolutly right, We need to count global number of MySqlSession instances. If the number of instances falls to zero we need to call mysql_library_end().

I use a memory profiler called valgrind. I invoke him with following command:
valgrind --tool=memcheck --leak-check=full --show-reachable=yes ./ProgramName


I enclose completed patch for this issue. I have added new static variable to count number of instances (numberOfInstances) of the class MySqlSession.

Sincerely,
Klugier



I believe that your code does not really do anything with numberOfInstances - it is always zero.

Anyway, while not quite sure about what mysql_library_end does, what about simply putting it to EXITBLOCK and call when any MySql was performed to keep valgrind quiet?

Mirek
Re: [BUG] Memory leak in MySql package [message #39925 is a reply to message #39921] Sat, 11 May 2013 17:19 Go to previous messageGo to next message
Klugier is currently offline  Klugier
Messages: 821
Registered: September 2012
Location: Poland, Kraków
Experienced Contributor
Hello Mirek,

I sorry, I have sent wrong header file. Of course, we need to add "counter" code to constructor and destructor.

MySqlSession()       { numberOfInstances++; mysql = NULL; Dialect(MY_SQL); }
~MySqlSession()      { numberOfInstances--; Close(); }


I would like to relate to putting EXITBLOCK. In my option counter method is more efficient, because memory can be free at runtime. On the other hand, EXITBLOCK will make order when the program ends. Of course, I can be wrong what EXITBLOCK is doing. I have never used it in my applications.

I have attached improved version of this patch.

Sincerely,
Klugier


Ultimate++ - one framework to rule them all.
Re: [BUG] Memory leak in MySql package [message #39981 is a reply to message #39925] Wed, 22 May 2013 09:41 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13032
Registered: November 2005
Ultimate Member
klugier wrote on Sat, 11 May 2013 11:19

Hello Mirek,

I sorry, I have sent wrong header file. Of course, we need to add "counter" code to constructor and destructor.

MySqlSession()       { numberOfInstances++; mysql = NULL; Dialect(MY_SQL); }
~MySqlSession()      { numberOfInstances--; Close(); }


I would like to relate to putting EXITBLOCK. In my option counter method is more efficient, because memory can be free at runtime. On the other hand, EXITBLOCK will make order when the program ends. Of course, I can be wrong what EXITBLOCK is doing. I have never used it in my applications.



Well, I think that amount of memory is not high and there is plausible usage scenario where you create DB connection per request - in that case MySql will have to recreate the whole memory area....

Frankly, more I am thinking about this, perhaps the best course of action is to do nothing. This is a 'leak' we can afford.

Mirek
Re: [BUG] Memory leak in MySql package [message #39982 is a reply to message #39611] Wed, 22 May 2013 10:13 Go to previous messageGo to next message
Alboni is currently offline  Alboni
Messages: 203
Registered: January 2012
Location: Deventer, Netherlands
Experienced Member
I'm in favor of doing the obvious:

mysql_library_init() in initblock
mysql_library_end() in exitblock

Re: [BUG] Memory leak in MySql package [message #39983 is a reply to message #39982] Wed, 22 May 2013 10:40 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 13032
Registered: November 2005
Ultimate Member
Alboni wrote on Wed, 22 May 2013 04:13


mysql_library_end() in exitblock



But what is the point?
Re: [BUG] Memory leak in MySql package [message #39985 is a reply to message #39611] Wed, 22 May 2013 11:53 Go to previous messageGo to next message
Alboni is currently offline  Alboni
Messages: 203
Registered: January 2012
Location: Deventer, Netherlands
Experienced Member
The point is that the MySql documentation says that mysql_library_end() should be called before closing the program. MySql was written by smart people and I see no reason to doubt their wisdom. I don't know what problems not calling it induces. Maybe none. Maybe some that are not evident. I haven't analysed MySql sourcecode.

[Updated on: Wed, 22 May 2013 12:04]

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39994 is a reply to message #39985] Thu, 23 May 2013 12:52 Go to previous messageGo to next message
piotr5 is currently offline  piotr5
Messages: 103
Registered: November 2005
Experienced Member
once upon a time closing a program that's using a database without closing that database did damage the actual database-file. one had to restore its contents with an external program. so were the times of msdos. with the introduction of write-caching also file-systems needed to be unmounted before turning off the computer. now filesystems have a journal for this purpose (even though still one is supposed to unmount them since otherwise the write-cache is lost). guess mysql is using similar solutions to avoid damage to data? maybe there's also some sort of write-cache for complicated meta-data that could become simplified in case large amounts of information is available? and if not, maybe the future would give us such technology...
Re: [BUG] Memory leak in MySql package [message #39995 is a reply to message #39611] Thu, 23 May 2013 13:17 Go to previous messageGo to next message
Alboni is currently offline  Alboni
Messages: 203
Registered: January 2012
Location: Deventer, Netherlands
Experienced Member
MySql is a separate program (unlike SqLite). It is not closed when the application program that uses it closes. So this will not damage the databasefile, but I can imagine that it can take longer for MySql to register that the client app is gone wich might be a problem if there is a maximum number of connections allowed etc. etc. Anyway can't figure out a reason not to do the closing call.
Re: [BUG] Memory leak in MySql package [message #40026 is a reply to message #39982] Sun, 26 May 2013 20:18 Go to previous message
mirek is currently offline  mirek
Messages: 13032
Registered: November 2005
Ultimate Member
Alboni wrote on Wed, 22 May 2013 04:13

I'm in favor of doing the obvious:

mysql_library_init() in initblock
mysql_library_end() in exitblock




So be it...

Mirek
Previous Topic: Reason why session does not connect
Next Topic: Sql(SqlSession& session) problem
Goto Forum:
  


Current Time: Sat Jan 16 10:23:00 CET 2021

Total time taken to generate the page: 0.02714 seconds