From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4551C44184A for ; Wed, 25 Mar 2020 04:43:13 +0300 (MSK) From: Nikita Pettik Date: Wed, 25 Mar 2020 04:43:02 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v2 06/10] box: introduce stacked diagnostic area List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org In terms of implementation, now struct error objects can be organized into double-linked lists. To achieve this pointers to the next and previous elements (cause and effort correspondingly) have been added to struct error. It is worth mentioning that already existing rlist and stailq list implementations are not suitable: rlist is cycled list, as a result it is impossible to start iteration over the list from random list entry and finish it at the logical end of the list; stailq is single-linked list leaving no possibility to remove elements from the middle of the list. As a part of C interface, box_error_add() has been introduced. In contrast to box_error_set() it does not replace last raised error, but instead it adds error to the list of diagnostic errors having already been set. If error is to be deleted (its reference counter hits 0 value) it is unlinked from the list it belongs to and destroyed. Meanwhile, error destruction leads to decrement of reference counter of its previous error and so on. To organize errors into lists in Lua, table representing error object in Lua now has .prev field (corresponding to 'previous' error) and method :set_prev(e). The latter accepts error object (i.e. created via box.error.new() or box.error.last()) and nil value. Both field .prev and :set_prev() method are implemented as ffi functions. Also note that cycles are not allowed while organizing errors into lists: e1 -> e2 -> e3; e3:set_prev(e1) -- would lead to error. Part of #1148 --- extra/exports | 1 + src/box/key_list.c | 4 +- src/box/lua/call.c | 4 +- src/lib/core/diag.c | 39 +++++ src/lib/core/diag.h | 96 ++++++++++- src/lib/core/exception.cc | 1 + src/lua/error.lua | 32 ++++ test/box/error.result | 284 ++++++++++++++++++++++++++++++++ test/box/error.test.lua | 105 ++++++++++++ test/engine/func_index.result | 6 - test/engine/func_index.test.lua | 1 - 11 files changed, 559 insertions(+), 14 deletions(-) diff --git a/extra/exports b/extra/exports index cbb5adcf4..9323996c1 100644 --- a/extra/exports +++ b/extra/exports @@ -234,6 +234,7 @@ box_error_message box_error_last box_error_clear box_error_set +error_set_prev box_latch_new box_latch_delete box_latch_lock diff --git a/src/box/key_list.c b/src/box/key_list.c index 3d736b55f..81eb501a5 100644 --- a/src/box/key_list.c +++ b/src/box/key_list.c @@ -64,7 +64,7 @@ key_list_iterator_create(struct key_list_iterator *it, struct tuple *tuple, /* Can't evaluate function. */ struct space *space = space_by_id(index_def->space_id); diag_set(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, - space ? space_name(space) : "", + space != NULL ? space_name(space) : "", diag_last_error(diag_get())->errmsg); return -1; } @@ -75,7 +75,7 @@ key_list_iterator_create(struct key_list_iterator *it, struct tuple *tuple, struct space *space = space_by_id(index_def->space_id); /* Can't get a result returned by function . */ diag_set(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, - space ? space_name(space) : "", + space != NULL ? space_name(space) : "", diag_last_error(diag_get())->errmsg); return -1; } diff --git a/src/box/lua/call.c b/src/box/lua/call.c index f1bbde7f0..92575374d 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -688,8 +688,8 @@ func_persistent_lua_load(struct func_lua *func) if (prepare_lua_sandbox(tarantool_L, default_sandbox_exports, nelem(default_sandbox_exports)) != 0) { diag_set(ClientError, ER_LOAD_FUNCTION, - func->base.def->name, - diag_last_error(diag_get())->errmsg); + func->base.def->name, + diag_last_error(diag_get())->errmsg); goto end; } } else { diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c index c350abb4a..57da5da44 100644 --- a/src/lib/core/diag.c +++ b/src/lib/core/diag.c @@ -34,6 +34,43 @@ /* Must be set by the library user */ struct error_factory *error_factory = NULL; +int +error_set_prev(struct error *e, struct error *prev) +{ + /* + * Make sure that adding error won't result in cycles. + * Don't bother with sophisticated cycle-detection + * algorithms, simple iteration is OK since as a rule + * list contains a dozen errors at maximum. + */ + struct error *tmp = prev; + while (tmp != NULL) { + if (tmp == e) + return -1; + tmp = tmp->cause; + } + /* + * At once error can feature only one reason. + * So unlink previous 'cause' node. + */ + if (e->cause != NULL) { + e->cause->effect = NULL; + error_unref(e->cause); + } + /* Set new 'prev' node. */ + e->cause = prev; + /* + * Unlink new 'effect' node from its old list of 'cause' + * errors. nil can be also passed as an argument. + */ + if (prev != NULL) { + error_unlink_effect(prev); + prev->effect = e; + error_ref(prev); + } + return 0; +} + void error_create(struct error *e, error_f destroy, error_f raise, error_f log, @@ -53,6 +90,8 @@ error_create(struct error *e, e->line = 0; } e->errmsg[0] = '\0'; + e->cause = NULL; + e->effect = NULL; } struct diag * diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h index 7e1e1a174..675b9c6f1 100644 --- a/src/lib/core/diag.h +++ b/src/lib/core/diag.h @@ -84,6 +84,23 @@ struct error { char file[DIAG_FILENAME_MAX]; /* Error description. */ char errmsg[DIAG_ERRMSG_MAX]; + /** + * Link to the cause and effect of given error. The cause + * creates the effect: + * e1 = box.error.new({code = 0, reason = 'e1'}) + * e2 = box.error.new({code = 0, reason = 'e2'}) + * e1:set_prev(e2) -- Now e2 is the cause of e1 and e1 is + * the effect of e2. + * Only cause keeps reference to avoid cyclic dependence. + * RLIST implementation is not really suitable here + * since it is organized as circular list. In such + * a case it is impossible to start an iteration + * from any node and finish at the logical end of the + * list. Double-linked list is required to allow deletion + * from the middle of the list. + */ + struct error *cause; + struct error *effect; }; static inline void @@ -96,11 +113,56 @@ static inline void error_unref(struct error *e) { assert(e->refs > 0); - --e->refs; - if (e->refs == 0) - e->destroy(e); + struct error *to_delete = e; + while (--to_delete->refs == 0) { + /* Unlink error from lists completely.*/ + struct error *cause = to_delete->cause; + if (to_delete->effect != NULL) + to_delete->effect->cause = to_delete->cause; + if (to_delete->cause != NULL) + to_delete->cause->effect = to_delete->effect; + to_delete->cause = NULL; + to_delete->effect = NULL; + to_delete->destroy(to_delete); + if (cause == NULL) + return; + to_delete = cause; + } +} + +/** + * Unlink error from its effect. For instance: + * e1 -> e2 -> e3 -> e4 (e1:set_prev(e2); e2:set_prev(e3) ...) + * unlink(e3): e1 -> e2 -> NULL; e3 -> e4 -> NULL + */ +static inline void +error_unlink_effect(struct error *e) +{ + if (e->effect != NULL) { + assert(e->refs > 1); + error_unref(e); + e->effect->cause = NULL; + } + e->effect = NULL; } +/** + * Set previous error: cut @a prev from its previous 'tail' of + * causes and link to the one @a e belongs to. Note that all + * previous errors starting from @a prev->cause are transferred + * with it as well (i.e. causes for given error are not erased). + * For instance: + * e1 -> e2 -> NULL; e3 -> e4 -> NULL; + * e2:set_effect(e3): e1 -> e2 -> e3 -> e4 -> NULL + * + * @a effect can be NULL. To be used as ffi method in lua/error.lua. + * + * @retval -1 in case adding @a effect results in list cycles; + * 0 otherwise. + */ +int +error_set_prev(struct error *e, struct error *prev); + NORETURN static inline void error_raise(struct error *e) { @@ -178,6 +240,25 @@ diag_set_error(struct diag *diag, struct error *e) assert(e != NULL); error_ref(e); diag_clear(diag); + error_unlink_effect(e); + diag->last = e; +} + +/** + * Add a new error to the diagnostics area. It is added to the + * tail, so that list forms stack. + * @param diag Diagnostics area. + * @param e Error to be added. + */ +static inline void +diag_add_error(struct diag *diag, struct error *e) +{ + assert(e != NULL); + error_ref(e); + error_unlink_effect(e); + e->cause = diag->last; + if (diag->last != NULL) + diag->last->effect = e; diag->last = e; } @@ -280,6 +361,15 @@ BuildSocketError(const char *file, unsigned line, const char *socketname, errno = save_errno; \ } while (0) +#define diag_add(class, ...) do { \ + int save_errno = errno; \ + say_debug("%s at %s:%i", #class, __FILE__, __LINE__); \ + struct error *e; \ + e = Build##class(__FILE__, __LINE__, ##__VA_ARGS__); \ + diag_add_error(diag_get(), e); \ + errno = save_errno; \ +} while (0) + #if defined(__cplusplus) } /* extern "C" */ #endif /* defined(__cplusplus) */ diff --git a/src/lib/core/exception.cc b/src/lib/core/exception.cc index 0ab10c4bd..180cb0e97 100644 --- a/src/lib/core/exception.cc +++ b/src/lib/core/exception.cc @@ -114,6 +114,7 @@ Exception::~Exception() if (this != &out_of_memory) { assert(refs == 0); } + TRASH((struct error *) this); } Exception::Exception(const struct type_info *type_arg, const char *file, diff --git a/src/lua/error.lua b/src/lua/error.lua index 7f249864a..bdc9c714d 100644 --- a/src/lua/error.lua +++ b/src/lua/error.lua @@ -24,12 +24,17 @@ struct error { char _file[DIAG_FILENAME_MAX]; /* Error description. */ char _errmsg[DIAG_ERRMSG_MAX]; + struct error *_cause; + struct error *_effect; }; char * exception_get_string(struct error *e, const struct method_info *method); int exception_get_int(struct error *e, const struct method_info *method); + +int +error_set_prev(struct error *e, struct error *prev); ]] local REFLECTION_CACHE = {} @@ -95,11 +100,37 @@ local function error_errno(err) return e end +local function error_prev(err) + local e = err._cause; + if e ~= nil then + return e + else + return nil + end +end + +local function error_set_prev(err, prev) + -- First argument must be error. + if not ffi.istype('struct error', err) then + error("Usage: error1:set_prev(error2)") + end + -- Second argument must be error or nil. + if not ffi.istype('struct error', prev) and prev ~= nil then + error("Usage: error1:set_prev(error2)") + end + local ok = ffi.C.error_set_prev(err, prev); + if ok ~= 0 then + error("Cycles are not allowed") + end + +end + local error_fields = { ["type"] = error_type; ["message"] = error_message; ["trace"] = error_trace; ["errno"] = error_errno; + ["prev"] = error_prev; } local function error_unpack(err) @@ -143,6 +174,7 @@ local error_methods = { ["raise"] = error_raise; ["match"] = error_match; -- Tarantool 1.6 backward compatibility ["__serialize"] = error_serialize; + ["set_prev"] = error_set_prev; } local function error_index(err, key) diff --git a/test/box/error.result b/test/box/error.result index a19a7044f..ff2b8b270 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -502,6 +502,290 @@ box.error() | --- | ... +-- gh-1148: errors can be arranged into list (so called +-- stacked diagnostics). +-- +e1 = box.error.new({code = 111, reason = "cause"}) + | --- + | ... +assert(e1.prev == nil) + | --- + | - true + | ... +e1:set_prev(e1) + | --- + | - error: 'builtin/error.lua: Cycles are not allowed' + | ... +assert(e1.prev == nil) + | --- + | - true + | ... +e2 = box.error.new({code = 111, reason = "cause of cause"}) + | --- + | ... +e1:set_prev(e2) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +e2:set_prev(e1) + | --- + | - error: 'builtin/error.lua: Cycles are not allowed' + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +-- At this point stack is following: e1 -> e2 +-- Let's test following cases: +-- 1. e3 -> e2, e1 -> NULL (e3:set_prev(e2)) +-- 2. e1 -> e3, e2 -> NULL (e1:set_prev(e3)) +-- 3. e3 -> e1 -> e2 (e3:set_prev(e1)) +-- 4. e1 -> e2 -> e3 (e2:set_prev(e3)) +-- +e3 = box.error.new({code = 111, reason = "another cause"}) + | --- + | ... +e3:set_prev(e2) + | --- + | ... +assert(e3.prev == e2) + | --- + | - true + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e1.prev == nil) + | --- + | - true + | ... + +-- Reset stack to e1 -> e2 and test case 2. +-- +e1:set_prev(e2) + | --- + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e3.prev == nil) + | --- + | - true + | ... +e1:set_prev(e3) + | --- + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e1.prev == e3) + | --- + | - true + | ... +assert(e3.prev == nil) + | --- + | - true + | ... + +-- Reset stack to e1 -> e2 and test case 3. +-- +e1:set_prev(e2) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e3.prev == nil) + | --- + | - true + | ... +e3:set_prev(e1) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e3.prev == e1) + | --- + | - true + | ... + +-- Unlink errors and test case 4. +-- +e1:set_prev(nil) + | --- + | ... +e2:set_prev(nil) + | --- + | ... +e3:set_prev(nil) + | --- + | ... +e1:set_prev(e2) + | --- + | ... +e2:set_prev(e3) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.prev == e3) + | --- + | - true + | ... +assert(e3.prev == nil) + | --- + | - true + | ... + +-- Test circle detecting. At the moment stack is +-- following: e1 -> e2 -> e3 +-- +e3:set_prev(e1) + | --- + | - error: 'builtin/error.lua: Cycles are not allowed' + | ... +assert(e3.prev == nil) + | --- + | - true + | ... +e3:set_prev(e2) + | --- + | - error: 'builtin/error.lua: Cycles are not allowed' + | ... +assert(e3.prev == nil) + | --- + | - true + | ... + +-- Test splitting list into two ones. +-- After that we will get two lists: e1->e2->e5 and e3->e4 +-- +e4 = box.error.new({code = 111, reason = "yet another cause"}) + | --- + | ... +e5 = box.error.new({code = 111, reason = "and another one"}) + | --- + | ... +e3:set_prev(e4) + | --- + | ... +e2:set_prev(e5) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.prev == e5) + | --- + | - true + | ... +assert(e3.prev == e4) + | --- + | - true + | ... +assert(e5.prev == nil) + | --- + | - true + | ... +assert(e4.prev == nil) + | --- + | - true + | ... + +-- Another splitting option: e1->e2 and e5->e3->e4 +-- But firstly restore to one single list e1->e2->e3->e4 +-- +e2:set_prev(e3) + | --- + | ... +e5:set_prev(e3) + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.prev == nil) + | --- + | - true + | ... +assert(e5.prev == e3) + | --- + | - true + | ... +assert(e3.prev == e4) + | --- + | - true + | ... +assert(e4.prev == nil) + | --- + | - true + | ... + +-- In case error is destroyed, it unrefs reference counter +-- of its previous error. In turn, box.error.clear() refs/unrefs +-- only head and doesn't touch other errors. +-- +e2:set_prev(nil) + | --- + | ... +box.error.set(e1) + | --- + | ... +assert(box.error.last() == e1) + | --- + | - true + | ... +assert(box.error.last().prev == e2) + | --- + | - true + | ... +box.error.clear() + | --- + | ... +assert(box.error.last() == nil) + | --- + | - true + | ... +assert(e1.prev == e2) + | --- + | - true + | ... +assert(e2.code == 111) + | --- + | - true + | ... +box.error.set(e1) + | --- + | ... +box.error.clear() + | --- + | ... +assert(e1.prev == e2) + | --- + | - true + | ... + space:drop() | --- | ... diff --git a/test/box/error.test.lua b/test/box/error.test.lua index a0b7d3e78..1fdd6ed98 100644 --- a/test/box/error.test.lua +++ b/test/box/error.test.lua @@ -108,4 +108,109 @@ box.error.new(err) box.error.clear() box.error() +-- gh-1148: errors can be arranged into list (so called +-- stacked diagnostics). +-- +e1 = box.error.new({code = 111, reason = "cause"}) +assert(e1.prev == nil) +e1:set_prev(e1) +assert(e1.prev == nil) +e2 = box.error.new({code = 111, reason = "cause of cause"}) +e1:set_prev(e2) +assert(e1.prev == e2) +e2:set_prev(e1) +assert(e2.prev == nil) +-- At this point stack is following: e1 -> e2 +-- Let's test following cases: +-- 1. e3 -> e2, e1 -> NULL (e3:set_prev(e2)) +-- 2. e1 -> e3, e2 -> NULL (e1:set_prev(e3)) +-- 3. e3 -> e1 -> e2 (e3:set_prev(e1)) +-- 4. e1 -> e2 -> e3 (e2:set_prev(e3)) +-- +e3 = box.error.new({code = 111, reason = "another cause"}) +e3:set_prev(e2) +assert(e3.prev == e2) +assert(e2.prev == nil) +assert(e1.prev == nil) + +-- Reset stack to e1 -> e2 and test case 2. +-- +e1:set_prev(e2) +assert(e2.prev == nil) +assert(e3.prev == nil) +e1:set_prev(e3) +assert(e2.prev == nil) +assert(e1.prev == e3) +assert(e3.prev == nil) + +-- Reset stack to e1 -> e2 and test case 3. +-- +e1:set_prev(e2) +assert(e1.prev == e2) +assert(e2.prev == nil) +assert(e3.prev == nil) +e3:set_prev(e1) +assert(e1.prev == e2) +assert(e2.prev == nil) +assert(e3.prev == e1) + +-- Unlink errors and test case 4. +-- +e1:set_prev(nil) +e2:set_prev(nil) +e3:set_prev(nil) +e1:set_prev(e2) +e2:set_prev(e3) +assert(e1.prev == e2) +assert(e2.prev == e3) +assert(e3.prev == nil) + +-- Test circle detecting. At the moment stack is +-- following: e1 -> e2 -> e3 +-- +e3:set_prev(e1) +assert(e3.prev == nil) +e3:set_prev(e2) +assert(e3.prev == nil) + +-- Test splitting list into two ones. +-- After that we will get two lists: e1->e2->e5 and e3->e4 +-- +e4 = box.error.new({code = 111, reason = "yet another cause"}) +e5 = box.error.new({code = 111, reason = "and another one"}) +e3:set_prev(e4) +e2:set_prev(e5) +assert(e1.prev == e2) +assert(e2.prev == e5) +assert(e3.prev == e4) +assert(e5.prev == nil) +assert(e4.prev == nil) + +-- Another splitting option: e1->e2 and e5->e3->e4 +-- But firstly restore to one single list e1->e2->e3->e4 +-- +e2:set_prev(e3) +e5:set_prev(e3) +assert(e1.prev == e2) +assert(e2.prev == nil) +assert(e5.prev == e3) +assert(e3.prev == e4) +assert(e4.prev == nil) + +-- In case error is destroyed, it unrefs reference counter +-- of its previous error. In turn, box.error.clear() refs/unrefs +-- only head and doesn't touch other errors. +-- +e2:set_prev(nil) +box.error.set(e1) +assert(box.error.last() == e1) +assert(box.error.last().prev == e2) +box.error.clear() +assert(box.error.last() == nil) +assert(e1.prev == e2) +assert(e2.code == 111) +box.error.set(e1) +box.error.clear() +assert(e1.prev == e2) + space:drop() diff --git a/test/engine/func_index.result b/test/engine/func_index.result index 84cb83022..159158f1c 100644 --- a/test/engine/func_index.result +++ b/test/engine/func_index.result @@ -261,12 +261,6 @@ box.schema.func.create('runtimeerror', {body = lua_code, is_deterministic = true idx = s:create_index('idx', {func = box.func.runtimeerror.id, parts = {{1, 'string'}}}) | --- | ... -s:insert({1}) - | --- - | - error: 'Failed to build a key for functional index ''idx'' of space ''withdata'': - | [string "return function(tuple) local ..."]:1: attempt to call - | global ''require'' (a nil value)' - | ... idx:drop() | --- | ... diff --git a/test/engine/func_index.test.lua b/test/engine/func_index.test.lua index f31162c97..c3c3a7029 100644 --- a/test/engine/func_index.test.lua +++ b/test/engine/func_index.test.lua @@ -98,7 +98,6 @@ lua_code = [[function(tuple) test_run:cmd("setopt delimiter ''"); box.schema.func.create('runtimeerror', {body = lua_code, is_deterministic = true, is_sandboxed = true}) idx = s:create_index('idx', {func = box.func.runtimeerror.id, parts = {{1, 'string'}}}) -s:insert({1}) idx:drop() -- Remove old persistent functions -- 2.17.1