Re: [Maria-developers] [Commits] 9f863a15b03: MDEV-11602 InnoDB leaks foreign key metadata on DDL operations

19 Dec 2016

ok to push.

Most likely the double free was result of some other bug fixed later.

R: Jan

On Mon, Dec 19, 2016 at 4:00 PM,  wrote:
...
revision-id: 9f863a15b037c0ddffc2d12c1f7d841466f9f345 (mariadb-10.0.28-27-
g9f863a15b03)
parent(s): eb4f2e063c341d9f3644339c68cb01679e782001
author: Marko Mäkelä
committer: Marko Mäkelä
timestamp: 2016-12-19 15:57:41 +0200
message:
MDEV-11602 InnoDB leaks foreign key metadata on DDL operations
Essentially revert MDEV-6759, which addressed a double free of memory
by removing the freeing altogether, introducing the memory leaks.
No double free was observed when running the test suite -DWITH_ASAN.
Replace some mem_heap_free(foreign->heap) with dict_foreign_free(foreign)
so that the calls can be located and instrumented more easily when needed.
---
 storage/innobase/dict/dict0dict.cc | 8 ++++----
 storage/innobase/dict/dict0load.cc | 2 +-
 storage/xtradb/dict/dict0dict.cc   | 8 ++++----
 storage/xtradb/dict/dict0load.cc   | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/
dict0dict.cc
index f75d584e70d..2e40946224e 100644
--- a/storage/innobase/dict/dict0dict.cc
+++ b/storage/innobase/dict/dict0dict.cc
@@ -1614,6 +1614,7 @@ struct dict_foreign_remove_partial
                if (table != NULL) {
                        table->referenced_set.erase(foreign);
                }
+               dict_foreign_free(foreign);
        }
 };
@@ -3535,8 +3536,7 @@ dict_foreign_add_to_cache(
        }
if (for_in_cache) {
-               /* Free the foreign object */
-               mem_heap_free(foreign->heap);
+               dict_foreign_free(foreign);
        } else {
                for_in_cache = foreign;
        }
@@ -3564,7 +3564,7 @@ dict_foreign_add_to_cache(
                                " the ones in table.");
if (for_in_cache == foreign) {
-                               mem_heap_free(foreign->heap);
+                               dict_foreign_free(foreign);
                        }
return(DB_CANNOT_ADD_CONSTRAINT);
@@ -3620,7 +3620,7 @@ dict_foreign_add_to_cache(
                                                        be one */
                                }
-                               mem_heap_free(foreign->heap);
+                               dict_foreign_free(foreign);
                        }
return(DB_CANNOT_ADD_CONSTRAINT);
diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/
dict0load.cc
index e68bbe7d10a..77a31e5de63 100644
--- a/storage/innobase/dict/dict0load.cc
+++ b/storage/innobase/dict/dict0load.cc
@@ -489,7 +489,7 @@ dict_process_sys_foreign_rec(
        }
/* This recieves a dict_foreign_t* that points to a stack variable.
-       So mem_heap_free(foreign->heap) is not used as elsewhere.
+       So dict_foreign_free(foreign) is not used as elsewhere.
        Since the heap used here is freed elsewhere, foreign->heap
        is not assigned. */
        foreign->id = mem_heap_strdupl(heap, (const char*) field, len);
diff --git a/storage/xtradb/dict/dict0dict.cc b/storage/xtradb/dict/
dict0dict.cc
index 5339ecfaed6..3845138c8bf 100644
--- a/storage/xtradb/dict/dict0dict.cc
+++ b/storage/xtradb/dict/dict0dict.cc
@@ -1614,6 +1614,7 @@ struct dict_foreign_remove_partial
                if (table != NULL) {
                        table->referenced_set.erase(foreign);
                }
+               dict_foreign_free(foreign);
        }
 };
@@ -3539,8 +3540,7 @@ dict_foreign_add_to_cache(
        }
if (for_in_cache) {
-               /* Free the foreign object */
-               mem_heap_free(foreign->heap);
+               dict_foreign_free(foreign);
        } else {
                for_in_cache = foreign;
        }
@@ -3564,7 +3564,7 @@ dict_foreign_add_to_cache(
                                " the ones in table.");
if (for_in_cache == foreign) {
-                               mem_heap_free(foreign->heap);
+                               dict_foreign_free(foreign);
                        }
return(DB_CANNOT_ADD_CONSTRAINT);
@@ -3620,7 +3620,7 @@ dict_foreign_add_to_cache(
                                                        be one */
                                }
-                               mem_heap_free(foreign->heap);
+                               dict_foreign_free(foreign);
                        }
return(DB_CANNOT_ADD_CONSTRAINT);
diff --git a/storage/xtradb/dict/dict0load.cc b/storage/xtradb/dict/
dict0load.cc
index ca7de72c9b9..9ea4f4d873a 100644
--- a/storage/xtradb/dict/dict0load.cc
+++ b/storage/xtradb/dict/dict0load.cc
@@ -491,7 +491,7 @@ dict_process_sys_foreign_rec(
        }
/* This recieves a dict_foreign_t* that points to a stack variable.
-       So mem_heap_free(foreign->heap) is not used as elsewhere.
+       So dict_foreign_free(foreign) is not used as elsewhere.
        Since the heap used here is freed elsewhere, foreign->heap
        is not assigned. */
        foreign->id = mem_heap_strdupl(heap, (const char*) field, len);
_______________________________________________
commits mailing list
commits@mariadb.org
https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

    

Re: [Maria-developers] [Commits] 9f863a15b03: MDEV-11602 InnoDB leaks foreign key metadata on DDL operations

Jan Lindström