* [Tarantool-patches] [PATCH 0/2] origin/2.2: add missing diag_set calls on error paths @ 2020-01-17 13:29 Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 1/2] box/request: add missing OutOfMemory diag_set Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure Cyrill Gorcunov 0 siblings, 2 replies; 6+ messages in thread From: Cyrill Gorcunov @ 2020-01-17 13:29 UTC (permalink / raw) To: tml Might be related to https://github.com/tarantool/tarantool/issues/4730 The series is on top of origin/2.2 Cyrill Gorcunov (2): box/request: add missing OutOfMemory diag_set box/applier: add missing diag_set on region_alloc failure src/box/applier.cc | 5 ++++- src/box/request.c | 8 ++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH 1/2] box/request: add missing OutOfMemory diag_set 2020-01-17 13:29 [Tarantool-patches] [PATCH 0/2] origin/2.2: add missing diag_set calls on error paths Cyrill Gorcunov @ 2020-01-17 13:29 ` Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure Cyrill Gorcunov 1 sibling, 0 replies; 6+ messages in thread From: Cyrill Gorcunov @ 2020-01-17 13:29 UTC (permalink / raw) To: tml In request_create_from_tuple and request_handle_sequence we may be unable to request memory for tuples, don't forget to setup diag error otherwise diag_raise will lead to nil dereference. Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> --- src/box/request.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/box/request.c b/src/box/request.c index 82232a155..994f2da62 100644 --- a/src/box/request.c +++ b/src/box/request.c @@ -109,8 +109,10 @@ request_create_from_tuple(struct request *request, struct space *space, * the tuple data to WAL on commit. */ char *buf = region_alloc(&fiber()->gc, size); - if (buf == NULL) + if (buf == NULL) { + diag_set(OutOfMemory, size, "region_alloc", "tuple"); return -1; + } memcpy(buf, data, size); request->tuple = buf; request->tuple_end = buf + size; @@ -199,8 +201,10 @@ request_handle_sequence(struct request *request, struct space *space) size_t buf_size = (request->tuple_end - request->tuple) + mp_sizeof_uint(UINT64_MAX); char *tuple = region_alloc(&fiber()->gc, buf_size); - if (tuple == NULL) + if (tuple == NULL) { + diag_set(OutOfMemory, buf_size, "region_alloc", "tuple"); return -1; + } char *tuple_end = mp_encode_array(tuple, len); if (unlikely(key != data)) { -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure 2020-01-17 13:29 [Tarantool-patches] [PATCH 0/2] origin/2.2: add missing diag_set calls on error paths Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 1/2] box/request: add missing OutOfMemory diag_set Cyrill Gorcunov @ 2020-01-17 13:29 ` Cyrill Gorcunov 2020-01-20 8:31 ` Konstantin Osipov 1 sibling, 1 reply; 6+ messages in thread From: Cyrill Gorcunov @ 2020-01-17 13:29 UTC (permalink / raw) To: tml In case if we're hitting memory limit allocting triggers we should setup diag error to prevent nil dereference in diag_raise call for example from applier_apply_tx. Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> --- src/box/applier.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/box/applier.cc b/src/box/applier.cc index 1a051e8c4..5b1efdabe 100644 --- a/src/box/applier.cc +++ b/src/box/applier.cc @@ -714,8 +714,11 @@ applier_apply_tx(struct stailq *rows) sizeof(struct trigger)); on_commit = (struct trigger *)region_alloc(&txn->region, sizeof(struct trigger)); - if (on_rollback == NULL || on_commit == NULL) + if (on_rollback == NULL || on_commit == NULL) { + diag_set(OutOfMemory, sizeof(struct trigger), + "region_alloc", "on_rollback/on_commit"); goto rollback; + } trigger_create(on_rollback, applier_txn_rollback_cb, NULL, NULL); txn_on_rollback(txn, on_rollback); -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure Cyrill Gorcunov @ 2020-01-20 8:31 ` Konstantin Osipov 2020-01-20 8:44 ` Cyrill Gorcunov 2020-01-20 10:59 ` Cyrill Gorcunov 0 siblings, 2 replies; 6+ messages in thread From: Konstantin Osipov @ 2020-01-20 8:31 UTC (permalink / raw) To: Cyrill Gorcunov; +Cc: tml * Cyrill Gorcunov <gorcunov@gmail.com> [20/01/17 16:32]: > In case if we're hitting memory limit allocting triggers > we should setup diag error to prevent nil dereference > in diag_raise call for example from applier_apply_tx. It used to be region_alloc_xc, which would throw exception. So it's a regression introduced by some of the "cleanup" patches. Could you please find when it was introduced? -- Konstantin Osipov, Moscow, Russia ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure 2020-01-20 8:31 ` Konstantin Osipov @ 2020-01-20 8:44 ` Cyrill Gorcunov 2020-01-20 10:59 ` Cyrill Gorcunov 1 sibling, 0 replies; 6+ messages in thread From: Cyrill Gorcunov @ 2020-01-20 8:44 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tml On Mon, Jan 20, 2020 at 11:31:48AM +0300, Konstantin Osipov wrote: > * Cyrill Gorcunov <gorcunov@gmail.com> [20/01/17 16:32]: > > In case if we're hitting memory limit allocting triggers > > we should setup diag error to prevent nil dereference > > in diag_raise call for example from applier_apply_tx. > > It used to be region_alloc_xc, which would throw exception. > > So it's a regression introduced by some of the "cleanup" patches. > > Could you please find when it was introduced? Will do. Still there are some other places where diag is not set so we're narrowing the real case now with help of testing runs the issue reporter kindly agreed to do. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure 2020-01-20 8:31 ` Konstantin Osipov 2020-01-20 8:44 ` Cyrill Gorcunov @ 2020-01-20 10:59 ` Cyrill Gorcunov 1 sibling, 0 replies; 6+ messages in thread From: Cyrill Gorcunov @ 2020-01-20 10:59 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tml On Mon, Jan 20, 2020 at 11:31:48AM +0300, Konstantin Osipov wrote: > * Cyrill Gorcunov <gorcunov@gmail.com> [20/01/17 16:32]: > > In case if we're hitting memory limit allocting triggers > > we should setup diag error to prevent nil dereference > > in diag_raise call for example from applier_apply_tx. > > It used to be region_alloc_xc, which would throw exception. > > So it's a regression introduced by some of the "cleanup" patches. > > Could you please find when it was introduced? According to git logs region_alloc_xc has never been used here, it indroduced by 8c84932ade1b70971890dfd27d72b3145834594b ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-20 10:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-17 13:29 [Tarantool-patches] [PATCH 0/2] origin/2.2: add missing diag_set calls on error paths Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 1/2] box/request: add missing OutOfMemory diag_set Cyrill Gorcunov 2020-01-17 13:29 ` [Tarantool-patches] [PATCH 2/2] box/applier: add missing diag_set on region_alloc failure Cyrill Gorcunov 2020-01-20 8:31 ` Konstantin Osipov 2020-01-20 8:44 ` Cyrill Gorcunov 2020-01-20 10:59 ` Cyrill Gorcunov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox