Bug #86244 Remove Sql_alloc
Submitted: 9 May 2017 14:03 Modified: 15 May 2017 14:30
Reporter: Steinar Gunderson Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Compiling Severity:S3 (Non-critical)
Version: OS:Any
Assigned to: CPU Architecture:Any

[9 May 2017 14:03] Steinar Gunderson 
Description:
We currently have a class Sql_alloc, which classes inherit to get operator new that automatically puts the class on the thread-local MEM_ROOT. This is wrong for several reasons:

1. It abuses public inheritance for something that is not an is-a relation.
2. It hides that the object goes on a MEM_ROOT for someone who only looks at the call site, creating surprising behavior.
3. It hides the use of thread-local storage, essentially creating a hidden dependency on a global variable.
4. It can make objects go onto the wrong MEM_ROOT, as the thread-local MEM_ROOT is not always the correct one.
5. It introduces a throwing operator new that returns nullptr, which is a violation of the C++ standard.

How to repeat:
N/A

Suggested fix:
1. Replace all existing calls of "new SomethingThatInheritsFromSqlAlloc()" to "new (*THR_MALLOC) SomethingThatInheritsFromSqlAlloc()".
2. Remove the implicit version of operator new from Sql_alloc, causing compile errors whenever the implicit version is called.
3. Wait until 8.0 release, making sure no new code is written that expects "new SomethingThatInheritsFromSqlAlloc()" to go on a MEM_ROOT is written.
4. Remove Sql_alloc itself, making "new SomethingThatInheritsFromSqlAlloc()" finally mean the same thing for every object.
[15 May 2017 14:30] Paul DuBois 
Posted by developer:
 
Fixed in 8.0.2.

Code cleanup. No changelog entry needed.
[12 Oct 2017 13:28] Paul DuBois 
Posted by developer:
 
Fixed in 9.0.0.

Code cleanup. No changelog entry needed.
[12 Feb 2018 20:18] Paul DuBois 
Posted by developer:
 
Fixed in 8.0.5, not 9.0.0.