[Tarantool-patches] [PATCH] error: use int64_t as reference counter
Dmitry Khominich
khdmitryi at gmail.com
Mon Jun 8 18:30:42 MSK 2020
Before it was unsafe to use error.prev, box.error.last(), or any error
method that increments error reference counter. Any of them could throw a
Lua error in case of INT32_MAX overflow.
This patch uses int64_t as error reference counter making it impossible
to get counter overflow.
Closes #4902
---
Branch: https://github.com/dmitryikh/tarantool/tree/error_ref_to_int64
Issue: https://github.com/tarantool/tarantool/issues/4902
Current patch is already reviewed here: https://github.com/tarantool/tarantool/pull/5018
src/exports.h | 1 +
src/lib/core/diag.c | 10 +++++++++-
src/lib/core/diag.h | 15 +++------------
src/lua/error.c | 3 +--
src/lua/error.lua | 16 ++++++----------
5 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/src/exports.h b/src/exports.h
index bffb1636b..c9eb31c99 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -114,6 +114,7 @@ EXPORT(csv_iterator_create)
EXPORT(csv_next)
EXPORT(csv_setopt)
EXPORT(decimal_unpack)
+EXPORT(error_ref)
EXPORT(error_set_prev)
EXPORT(error_unpack_unsafe)
EXPORT(error_unref)
diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c
index 90c4fa21a..bcb0d1f1c 100644
--- a/src/lib/core/diag.c
+++ b/src/lib/core/diag.c
@@ -31,6 +31,15 @@
#include "diag.h"
#include "fiber.h"
+void
+error_ref(struct error *e)
+{
+ assert(e->refs >= 0);
+ if (e->refs >= INT64_MAX)
+ panic("too many references to error object");
+ e->refs++;
+}
+
void
error_unref(struct error *e)
{
@@ -80,7 +89,6 @@ error_set_prev(struct error *e, struct error *prev)
*/
error_unlink_effect(prev);
}
- assert(prev->refs < INT32_MAX);
error_ref(prev);
prev->effect = e;
}
diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h
index ae49fd8b0..f68415f6f 100644
--- a/src/lib/core/diag.h
+++ b/src/lib/core/diag.h
@@ -80,7 +80,7 @@ struct error {
* meanwhile it is still used in C internals or vice
* versa. For details see luaT_pusherror().
*/
- int refs;
+ int64_t refs;
/**
* Errno at the moment of the error
* creation. If the error is not related
@@ -112,15 +112,8 @@ struct error {
struct error *effect;
};
-static inline int
-error_ref(struct error *e)
-{
- assert(e->refs >= 0);
- if (e->refs >= INT32_MAX)
- return -1;
- e->refs++;
- return 0;
-}
+void
+error_ref(struct error *e);
void
error_unref(struct error *e);
@@ -234,7 +227,6 @@ static inline void
diag_set_error(struct diag *diag, struct error *e)
{
assert(e != NULL);
- assert(e->refs < INT32_MAX);
error_ref(e);
diag_clear(diag);
error_unlink_effect(e);
@@ -259,7 +251,6 @@ diag_add_error(struct diag *diag, struct error *e)
*/
assert(e->effect == NULL);
assert(diag->last->effect == NULL);
- assert(e->refs < INT32_MAX);
error_ref(e);
e->cause = diag->last;
diag->last->effect = e;
diff --git a/src/lua/error.c b/src/lua/error.c
index 9fb2f56f0..616aa7f87 100644
--- a/src/lua/error.c
+++ b/src/lua/error.c
@@ -84,8 +84,7 @@ luaT_pusherror(struct lua_State *L, struct error *e)
* It also important to reference the error first and only
* then set the finalizer.
*/
- if (error_ref(e) != 0)
- luaL_error(L, "Too many references to error object");
+ error_ref(e);
assert(CTID_CONST_STRUCT_ERROR_REF != 0);
struct error **ptr = (struct error **)
luaL_pushcdata(L, CTID_CONST_STRUCT_ERROR_REF);
diff --git a/src/lua/error.lua b/src/lua/error.lua
index b2646cb01..ebc784f07 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -16,7 +16,7 @@ struct error {
error_f _raise;
error_f _log;
const struct type_info *_type;
- int _refs;
+ int64_t _refs;
int _saved_errno;
/** Line number. */
unsigned _line;
@@ -39,6 +39,9 @@ error_set_prev(struct error *e, struct error *prev);
const char *
box_error_custom_type(const struct error *e);
+void
+error_ref(struct error *e);
+
void
error_unref(struct error *e);
]]
@@ -117,11 +120,7 @@ end
local function error_prev(err)
local e = err._cause;
if e ~= nil then
- local INT32_MAX = 2147483647
- if e._refs >= INT32_MAX then
- error("Too many references to error object")
- end
- e._refs = e._refs + 1
+ ffi.C.error_ref(e)
e = ffi.gc(e, ffi.C.error_unref)
return e
else
@@ -138,10 +137,7 @@ local function error_set_prev(err, prev)
if not ffi.istype('struct error', prev) and prev ~= nil then
error("Usage: error1:set_prev(error2)")
end
- local INT32_MAX = 2147483647
- if err._refs >= INT32_MAX then
- error("Too many references to error object")
- end
+ ffi.C.error_ref(err)
local ok = ffi.C.error_set_prev(err, prev);
if ok ~= 0 then
error("Cycles are not allowed")
--
2.25.0
More information about the Tarantool-patches
mailing list