[Tarantool-patches] [PATCH] error: use int64_t as reference counter

[Tarantool-patches] [PATCH] error: use int64_t as reference counter

[Dmitry Khominich]

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

Re: [Tarantool-patches] [PATCH] error: use int64_t as reference counter

[Kirill Yukhin]

Hello,

On 08 июн 18:30, Dmitry Khominich wrote:
> 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

I've checked your patch into 2.4 and master.

--
Regards, Kirill Yukhin