From: Kirill Shcherbatov <kshcherbatov@tarantool.org> To: tarantool-patches@freelists.org, v.shpilevoy@tarantool.org Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org> Subject: [tarantool-patches] [PATCH v1 1/1] sql: Fix DEFAULTs AST memory leak on alter Date: Tue, 25 Dec 2018 15:00:33 +0300 [thread overview] Message-ID: <a9dc4eeafac2a1e4fb2fa368a01ad668fc116e5d.1545739120.git.kshcherbatov@tarantool.org> (raw) The space_def_destroy_fields routine is used in make_scoped_guard on alter:space_def_new_from_tuple always pass extern_alloc=true for sql_expr_delete routine. It shouldn't as the first-time allocated AST (field_def_decode) is not external-allocated. Introduced a new flag 'extern_alloc' for space_def_destroy_fields routine. Closes #3908 http://github.com/tarantool/tarantool/tree/kshch/gh-3908-fix-ast-alter-memleak https://github.com/tarantool/tarantool/issues/3908 --- src/box/alter.cc | 4 ++-- src/box/space_def.c | 7 ++++--- src/box/space_def.h | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 03ba68adc..5ec57590e 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -452,7 +452,7 @@ space_format_decode(const char *data, uint32_t *out_count, */ memset(region_defs, 0, size); auto fields_guard = make_scoped_guard([=] { - space_def_destroy_fields(region_defs, count); + space_def_destroy_fields(region_defs, count, false); }); for (uint32_t i = 0; i < count; ++i) { field_def_decode(®ion_defs[i], &data, space_name, name_len, @@ -515,7 +515,7 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode, fields = space_format_decode(format, &field_count, name, name_len, errcode, region); auto fields_guard = make_scoped_guard([=] { - space_def_destroy_fields(fields, field_count); + space_def_destroy_fields(fields, field_count, false); }); if (exact_field_count != 0 && exact_field_count < field_count) { diff --git a/src/box/space_def.c b/src/box/space_def.c index 69b92c5c1..3516bdd8d 100644 --- a/src/box/space_def.c +++ b/src/box/space_def.c @@ -259,12 +259,13 @@ space_def_new(uint32_t id, uint32_t uid, uint32_t exact_field_count, /** Free a default value's syntax trees of @a defs. */ void -space_def_destroy_fields(struct field_def *fields, uint32_t field_count) +space_def_destroy_fields(struct field_def *fields, uint32_t field_count, + bool extern_alloc) { for (uint32_t i = 0; i < field_count; ++i) { if (fields[i].default_value_expr != NULL) { sql_expr_delete(sql_get(), fields[i].default_value_expr, - true); + extern_alloc); } } } @@ -274,7 +275,7 @@ space_def_delete(struct space_def *def) { space_opts_destroy(&def->opts); tuple_dictionary_unref(def->dict); - space_def_destroy_fields(def->fields, def->field_count); + space_def_destroy_fields(def->fields, def->field_count, true); TRASH(def); free(def); } diff --git a/src/box/space_def.h b/src/box/space_def.h index 0d1e90233..e99581dab 100644 --- a/src/box/space_def.h +++ b/src/box/space_def.h @@ -122,9 +122,11 @@ struct space_def { * Free a default value syntax trees of @a defs. * @param fields Fields array to destroy. * @param field_count Length of @a fields. + * @param extern_alloc Fields expression AST allocated externally. */ void -space_def_destroy_fields(struct field_def *fields, uint32_t field_count); +space_def_destroy_fields(struct field_def *fields, uint32_t field_count, + bool extern_alloc); /** * Delete the space_def object. -- 2.19.2
next reply other threads:[~2018-12-25 12:00 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-12-25 12:00 Kirill Shcherbatov [this message] 2018-12-25 12:37 ` [tarantool-patches] " Kirill Shcherbatov 2018-12-25 12:38 ` Vladislav Shpilevoy 2018-12-25 13:29 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=a9dc4eeafac2a1e4fb2fa368a01ad668fc116e5d.1545739120.git.kshcherbatov@tarantool.org \ --to=kshcherbatov@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [tarantool-patches] [PATCH v1 1/1] sql: Fix DEFAULTs AST memory leak on alter' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox