maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Monty!

On Mar 28, Sergei Golubchik wrote:
> On Mar 28, Michael Widenius wrote:
> > On Sun, Mar 28, 2021 at 3:00 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
> > > On Mar 28, Michael Widenius wrote:
> > > > revision-id: dbb62d20f36 (mariadb-10.5.2-518-gdbb62d20f36)
> > > > parent(s): 0c37211c2a2
> > > > author: Michael Widenius <michael.widenius@xxxxxxxxx>
> > > > committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> > > > timestamp: 2021-03-24 14:31:53 +0200
> > > > message:
> > > >
> > > > Fixes that enables my_new.cc (new wrapper using my_malloc)
> > > >
> > > > This is not enabled by default as there are leaks in the server
> > > > that needs to be fixed first. One can compile with
> > > > -DREALLY_USE_MYSYS_NEW -DSF_REMEMBER_FRAMES=14 to find the memory
> > > > leaks from 'new'

Here's a simplified patch that works (I've applied it, compiled, run
the test suite, and verified in a debugger that new() from my_new.cc is
used):
==================
diff --git a/mysys/my_new.cc b/mysys/my_new.cc
index 88080f3e7eb..89869f6e0fa 100644
--- a/mysys/my_new.cc
+++ b/mysys/my_new.cc
@@ -25,34 +25,52 @@
 #include "mysys_priv.h"
 #include <new>
 
+/*
+  We don't yet enable the my new operators by default.
+  The reasons (for MariaDB) are:
+
+   - There are several global objects in plugins (wsrep_info, InnoDB,
+     tpool) that allocates data with 'new'. These objects are not
+     freed properly before exit() is called and safemalloc will report
+     these as lost memory.  The proper fix is to ensure that all plugins
+     either ensure that all objects frees there data or the global object are
+     changed to a pointer that as allocated and freed on demand.
+     Doing this will make it easier to find leaks and also speed up plugin
+     loads when we don't have to initialize a lot of objects until they
+     are really needed.
+  - Rocksdb calls malloc_usable_size, that will crash if used with new based
+    on my_malloc. One suggested fix would be to not define
+    ROCKSDB_MALLOC_USABLE_SIZE if MYSYS_USE_NEW is defined.
+*/
+
 #ifdef USE_MYSYS_NEW
 
 void *operator new (size_t sz)
 {
-  return (void *) my_malloc (sz ? sz : 1, MYF(0));
+  return (void *) my_malloc(key_memory_new, sz ? sz : 1, MYF(0));
 }
 
 void *operator new[] (size_t sz)
 {
-  return (void *) my_malloc (sz ? sz : 1, MYF(0));
+  return (void *) my_malloc(key_memory_new, sz ? sz : 1, MYF(0));
 }
 
 void* operator new(std::size_t sz, const std::nothrow_t&) throw()
 {
-  return (void *) my_malloc (sz ? sz : 1, MYF(0));
+  return (void *) my_malloc(key_memory_new, sz ? sz : 1, MYF(0));
 }
 
 void* operator new[](std::size_t sz, const std::nothrow_t&) throw()
 {
-  return (void *) my_malloc (sz ? sz : 1, MYF(0));
+  return (void *) my_malloc(key_memory_new, sz ? sz : 1, MYF(0));
 }
 
-void operator delete (void *ptr, std::size_t)
+void operator delete (void *ptr, std::size_t) throw ()
 {
   my_free(ptr);
 }
 
-void operator delete (void *ptr)
+void operator delete (void *ptr) throw ()
 {
   my_free(ptr);
 }
diff --git a/mysys/my_static.c b/mysys/my_static.c
index 6c09ab8d94b..1da04599793 100644
--- a/mysys/my_static.c
+++ b/mysys/my_static.c
@@ -48,6 +48,7 @@ PSI_memory_key key_memory_my_err_head;
 PSI_memory_key key_memory_my_file_info;
 PSI_memory_key key_memory_pack_frm;
 PSI_memory_key key_memory_charsets;
+PSI_memory_key key_memory_new;
 
 #ifdef _WIN32
 PSI_memory_key key_memory_win_SECURITY_ATTRIBUTES;
diff --git a/mysys/mysys_priv.h b/mysys/mysys_priv.h
index adf2d39046a..c5e05ec7955 100644
--- a/mysys/mysys_priv.h
+++ b/mysys/mysys_priv.h
@@ -88,6 +88,7 @@ extern PSI_memory_key key_memory_my_err_head;
 extern PSI_memory_key key_memory_my_file_info;
 extern PSI_memory_key key_memory_pack_frm;
 extern PSI_memory_key key_memory_charsets;
+extern PSI_memory_key key_memory_new;
 
 #ifdef _WIN32
 extern PSI_memory_key key_memory_win_SECURITY_ATTRIBUTES;
==================

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx