From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-f67.google.com (mail-pj1-f67.google.com [209.85.216.67]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 913A9469710 for ; Mon, 8 Jun 2020 18:30:58 +0300 (MSK) Received: by mail-pj1-f67.google.com with SMTP id q24so6157244pjd.1 for ; Mon, 08 Jun 2020 08:30:58 -0700 (PDT) From: Dmitry Khominich Date: Mon, 8 Jun 2020 18:30:42 +0300 Message-Id: <20200608153042.55274-1-khdmitryi@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] error: use int64_t as reference counter List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org 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