U++ forum: Welcome to the forum

Search on this site

Search in forums

Home » U++ Library support » U++ SQL » [BUG] Memory leak in MySql package

Show: Today's Messages :: Show Polls :: Message Navigator
E-mail to friend

[BUG] Memory leak in MySql package [message #39611]

Fri, 05 April 2013 12:43

Klugier
Messages: 1099
Registered: September 2012
Location: Poland, Kraków

Senior 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

U++ - 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

Alboni
Messages: 216
Registered: January 2012
Location: Kajaani, Finland

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).

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39719 is a reply to message #39714]

Sat, 20 April 2013 15:05

Klugier
Messages: 1099
Registered: September 2012
Location: Poland, Kraków

Senior 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

Attachment: MySql-Patch.tar.gz
(Size: 4.12KB, Downloaded 317 times)

U++ - 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

BioBytes
Messages: 308
Registered: October 2008
Location: France

Senior Member

Hello Klugier,

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

Regards

Biobytes

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39915 is a reply to message #39819]

Sat, 11 May 2013 02:31

Klugier
Messages: 1099
Registered: September 2012
Location: Poland, Kraków

Senior Contributor

Hello BioBytes,

Now, I am waiting for patch confirmation.

Sincerely,
Klugier

U++ - one framework to rule them all.

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39921 is a reply to message #39719]

Sat, 11 May 2013 11:27

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

klugier wrote on Sat, 20 April 2013 09:05

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

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39925 is a reply to message #39921]

Sat, 11 May 2013 17:19

Klugier
Messages: 1099
Registered: September 2012
Location: Poland, Kraków

Senior 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

Attachment: MySqlPatch2.tar.gz
(Size: 4.13KB, Downloaded 327 times)

U++ - one framework to rule them all.

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39981 is a reply to message #39925]

Wed, 22 May 2013 09:41

mirek
Messages: 14255
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(); }

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

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39982 is a reply to message #39611]

Wed, 22 May 2013 10:13

Alboni
Messages: 216
Registered: January 2012
Location: Kajaani, Finland

Experienced Member

I'm in favor of doing the obvious:

mysql_library_init() in initblock
mysql_library_end() in exitblock

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39983 is a reply to message #39982]

Wed, 22 May 2013 10:40

mirek
Messages: 14255
Registered: November 2005

Ultimate Member

Alboni wrote on Wed, 22 May 2013 04:13

mysql_library_end() in exitblock

But what is the point?

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39985 is a reply to message #39611]

Wed, 22 May 2013 11:53

Alboni
Messages: 216
Registered: January 2012
Location: Kajaani, Finland

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

piotr5
Messages: 107
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...

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #39995 is a reply to message #39611]

Thu, 23 May 2013 13:17

Alboni
Messages: 216
Registered: January 2012
Location: Kajaani, Finland

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.

Report message to a moderator

Re: [BUG] Memory leak in MySql package [message #40026 is a reply to message #39982]

Sun, 26 May 2013 20:18

mirek
Messages: 14255
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

Report message to a moderator

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

Goto Forum:

-=] Back to Top [=-

[ Syndicate this forum (XML) ] [

] [

]

Current Time: Fri Apr 25 19:10:55 CEST 2025

Total time taken to generate the page: 0.01460 seconds