patch.txt
[MDEV-4570] [PATCH] Sys_query_cache_limit initialization depends on initialization in other source files Created: 2013-05-24 Updated: 2013-07-17 Resolved: 2013-07-17 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | None |
| Affects Version/s: | 10.0.2, 5.5.31 |
| Fix Version/s: | 10.0.4, 5.5.33 |
| Type: | Bug | Priority: | Major |
| Reporter: | Pavel Ivanov | Assignee: | Oleksandr Byelkin |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Attachments: |
patch.txt
|
| Description |
|
Sys_query_cache_limit variable has query_cache.query_cache_limit as a place to write its value to. But it's a member of a class that has constructor and so initialization of Sys_query_cache_limit (which assigns default value to the variable) is dependent on initialization of query_cache to happen earlier. As these variables are in different source files order of initialization between them is not defined. The attached patch fixes Sys_query_cache_limit to point to global variable instead of class member. Detected by new feature of AddressSanitizer – check_initialization_order. |
| Comments |
| Comment by Sergei Golubchik [ 2013-05-24 ] |
|
Sanja, please fix that in 5.5, if applicable. |
| Comment by Oleksandr Byelkin [ 2013-05-30 ] |
|
As I can see fix_query_cache_limit (as well other fix* functions) is not called on initialization with default value so I do not see how the patch can help except hiding the problem... maybe I am wrong, still checking... |
| Comment by Pavel Ivanov [ 2013-05-30 ] |
|
Actually if fix_query_cache_limit function will be called during initialization then it will cause the same problem again and patch won't fix anything. If fix_query_cache_limit functions is not called on initialization then the patch reaches its goal – query_cache and Sys_query_cach_limit are initialized independently from each other and don't touch each other's data during initialization. BTW, I noticed that query_cache initializes query_cache_limit to ULONG_MAX while Sys_query_cache_limit has maximum of UINT_MAX, so it may be worth checking what these values should be. |
| Comment by Oleksandr Byelkin [ 2013-05-31 ] |
|
I think setting result size limit could be moved to QC initialization, and this (with above patch) will solve the problem. |
| Comment by Sergei Golubchik [ 2013-05-31 ] |
|
Strictly speaking, there is no bug here. All command-line options (and query_cache_limit is a command-line option) are initialized run-time from handle_options() function of my_getopt.c. So whatever static initialization does and in what order is irrelevant. But okay, one can argue that it's a fragile assumption to rely on (if we'd decide to remove a possibility of setting query_cache_limit from the command line, it will make its initial value non-deterministic). Let's fix it. sanja — ok to push. |
| Comment by Oleksandr Byelkin [ 2013-07-16 ] |
|
pushed to 10.0 |