maria-developers team mailing list archive

[Jan Lindström <jan.lindstrom@xxxxxxxxxxx>]

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, <marko.makela@xxxxxxxxxxx> 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@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits