Bug #18930 | Memory leak in sp-vars.test | ||
---|---|---|---|
Submitted: | 10 Apr 2006 5:24 | Modified: | 8 May 2006 18:32 |
Reporter: | Ingo Strüwing | Email Updates: | |
Status: | Closed | Impact on me: | |
Category: | MySQL Server: Stored Routines | Severity: | S2 (Serious) |
Version: | 5.0.21, 5.1.10 | OS: | Linux (Linux) |
Assigned to: | Michael Widenius | CPU Architecture: | Any |
[10 Apr 2006 5:24]
Ingo Strüwing
[10 Apr 2006 14:34]
Valeriy Kravchuk
Thank you for a problem report. Verified just as described with 5.1-debug-BK (ChangeSet@1.2303.1.1, 2006-04-09 19:43:36-07:00): TEST RESULT ------------------------------------------------------- sp-vars [ pass ] ------------------------------------------------------- Ending Tests Shutting-down MySQL daemon Master shutdown finished Slave shutdown finished All 1 tests were successful. WARNING: Got errors/warnings while running tests. Please examine /home/openxs/dbs/5.1/mysql-test/var/log/warnings for details. openxs@suse:~/dbs/5.1/mysql-test> tail var/log/warnings Warning: Not freed memory segments: 56 Warning: Memory that was not free'ed (38912 bytes):
[18 Apr 2006 7:01]
Kristian Nielsen
Saw this problem in Valgrind as well. This is old code, so I would assume it is in 4.1 (and 4.0) as well, though have not checked. The problem occurs when the parser stack needs to be extended. We #define yyoverflow my_yyoverflow() for that, and use my_realloc() to extend the stack. However, when yyoverflow is defined, Bison does not itself deallocate the stack after parsing, and the MySQL code doesn't either -> 26,000 bytes leaked. The default parser stack is sufficient for most parsing, so this is not seen too often. The following patch fixes it and removes the need to use our own yyoverflow definition: --- sql/sql_yacc.yy.orig 2006-04-09 11:01:05.000000000 +0200 +++ sql/sql_yacc.yy 2006-04-13 09:39:59.000000000 +0200 @@ -45,7 +45,9 @@ const LEX_STRING null_lex_str={0,0}; -#define yyoverflow(A,B,C,D,E,F) {ulong val= *(F); if (my_yyoverflow((B), (D), &val)) { yyerror((char*) (A)); return 2; } else { *(F)= (YYSIZE_T)val; }} +#define YYMAXDEPTH 32000 +#define YYMALLOC(bytes) my_malloc(bytes, MYF(0)) +#define YYFREE(ptr) my_free(ptr, MYF(0)) #define WARN_DEPRECATED(A,B) \ push_warning_printf(((THD *)yythd), MYSQL_ERROR::WARN_LEVEL_WARN, \ However I would like to check a few things about this patch (required Bison version, clean up my_yyoverflow() etc, ...) to make sure it is a viable approach. Alternatively, we need to keep track ourself of the memory allocated by my_yyoverflow() and free it after parsing is done. Changing Category, since it is not really a stored procedure problem.
[19 Apr 2006 12:09]
Kristian Nielsen
This is actually a fairly serious leak; every client connection that executes this will leak 26,000 bytes: CREATE PROCEDURE p1() BEGIN DECLARE dummy TINYINT DEFAULT 0; CASE 0 WHEN 0 THEN SET dummy = 0; WHEN 1 THEN SET dummy = 0; WHEN 2 THEN SET dummy = 0; WHEN 3 THEN SET dummy = 0; WHEN 4 THEN SET dummy = 0; WHEN 5 THEN SET dummy = 0; WHEN 6 THEN SET dummy = 0; WHEN 7 THEN SET dummy = 0; WHEN 8 THEN SET dummy = 0; WHEN 9 THEN SET dummy = 0; ELSE SET dummy = 0; END CASE; END
[19 Apr 2006 14:20]
Kristian Nielsen
Unfortunately, it seems that the simple solution of just #defining YYMALLOC will not work for us (Bison will use alloca() in some cases, which I do not think we want; I've seen segfaults when the stack is grown in this case). So we need to find somewhere to store the memory returned by my_yyoverflow so that we can free it after parsing.
[19 Apr 2006 20:14]
Kristian Nielsen
Digged some more, the problem occurs also with views: CREATE VIEW v1 AS SELECT 1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(1+(0+0)))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))); SELECT * FROM v1; This leaks memory in the same was as the stored procedure examples. There is actually code to free the memory allocated by my_yyoverflow(), in lex_end() in sql_lex.cc. But for stored procedures and views, the call of this function seems to be missing: db_load_routine(), in sp.cc, calls lex_start() but not lex_end(). mysql_make_view(), in sql_view.cc, calls lex_start() but not lex_end(). sp_head::reset_lex(), in sp_head.cc, also calls lex_start() to push a new lex object on the m_lex list, but it is not clear to me whether that is subsequently called lex_end() upon. So the proper fix should be to make sure that lex_end() is called in the right places.
[5 May 2006 11:50]
Michael Widenius
Thank you for your bug report. This issue has been committed to our source repository of that product and will be incorporated into the next release. If necessary, you can access the source repository and build the latest available version, including the bugfix, yourself. More information about accessing the source trees is available at http://www.mysql.com/doc/en/Installing_source_tree.html Additional info: Fix will be in 5.0.22 and 5.1.10
[8 May 2006 18:32]
Paul DuBois
Noted in 5.0.22, 5.1.10 changelogs. The parser leaked memory when its stack needed to be extended. (Bug #18930)