[Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area
Nikita Pettik
korablev at tarantool.org
Mon Apr 6 17:17:10 MSK 2020
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 effect 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/lib/core/diag.c | 47 ++++++
src/lib/core/diag.h | 102 ++++++++++++-
src/lib/core/exception.cc | 1 +
src/lua/error.lua | 32 ++++
test/box/error.result | 303 ++++++++++++++++++++++++++++++++++++++
test/box/error.test.lua | 112 ++++++++++++++
7 files changed, 595 insertions(+), 3 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/lib/core/diag.c b/src/lib/core/diag.c
index 8352cbdf7..e143db18d 100644
--- a/src/lib/core/diag.c
+++ b/src/lib/core/diag.c
@@ -31,6 +31,51 @@
#include "diag.h"
#include "fiber.h"
+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.
+ */
+ if (prev != NULL) {
+ if (e == prev)
+ return -1;
+ if (prev->effect != NULL || e->effect != NULL) {
+ /*
+ * e and prev are already compared, so start
+ * from prev->cause.
+ */
+ struct error *tmp = prev->cause;
+ while (tmp != NULL) {
+ if (tmp == e)
+ return -1;
+ tmp = tmp->cause;
+ }
+ /*
+ * Unlink new 'effect' node from its old
+ * list of 'cause' errors.
+ */
+ error_unlink_effect(prev);
+ }
+ error_ref(prev);
+ prev->effect = e;
+ }
+ /*
+ * 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;
+ return 0;
+}
+
void
error_create(struct error *e,
error_f destroy, error_f raise, error_f log,
@@ -50,6 +95,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..9c238f5a2 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;
+ assert(to_delete->effect == NULL);
+ if (to_delete->cause != NULL) {
+ to_delete->cause->effect = NULL;
+ to_delete->cause = 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,31 @@ 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);
+ /* diag_set_error() should be called before. */
+ assert(diag->last != NULL);
+ /*
+ * e should be the bottom of its own stack.
+ * Otherwise some errors may be lost.
+ */
+ assert(e->effect == NULL);
+ assert(diag->last->effect == NULL);
+ error_ref(e);
+ e->cause = diag->last;
+ diag->last->effect = e;
diag->last = e;
}
@@ -280,6 +367,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 0a7ec6553..3d07f6e64 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -505,3 +505,306 @@ box.error()
space:drop()
| ---
| ...
+
+-- 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
+ | ...
+
+-- Set middle of an error stack into the diagnostics area.
+e1:set_prev(e2)
+ | ---
+ | ...
+e2:set_prev(e3)
+ | ---
+ | ...
+box.error.set(e2)
+ | ---
+ | ...
+assert(e1.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == e3)
+ | ---
+ | - true
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index a0b7d3e78..ed7eb7565 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -109,3 +109,115 @@ box.error.clear()
box.error()
space:drop()
+
+-- 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)
+
+-- Set middle of an error stack into the diagnostics area.
+e1:set_prev(e2)
+e2:set_prev(e3)
+box.error.set(e2)
+assert(e1.prev == nil)
+assert(e2.prev == e3)
--
2.17.1
More information about the Tarantool-patches
mailing list