From 062aa90a1722177643055afdd028e6fd7051e1bd Mon Sep 17 00:00:00 2001
From: Alexandr <a.lyapunov@corp.mail.ru>
Date: Thu, 2 Oct 2014 14:16:35 +0400
Subject: [PATCH] fix gh-505
---
src/box/engine_memtx.cc | 2 +-
src/box/engine_sophia.cc | 2 +-
src/box/lua/schema.lua | 12 +++----
src/box/lua/tuple.cc | 4 +--
src/box/lua/tuple.lua | 13 +++++---
src/box/sophia_index.cc | 12 +++----
src/box/tuple.cc | 13 ++------
src/box/tuple.h | 69 ++++++++++++++++++++++++++++++++++++----
src/box/txn.cc | 8 ++---
src/errcode.h | 1 +
src/ffisyms.cc | 3 +-
test/big/lua.result | 2 +-
test/box/misc.result | 1 +
test/box/select.result | 29 +++++++++++++++++
test/box/select.test.lua | 10 ++++++
test/box/tuple.result | 22 ++++++-------
16 files changed, 147 insertions(+), 56 deletions(-)
diff --git a/src/box/engine_memtx.cc b/src/box/engine_memtx.cc
index ade14477d2..24f3094656 100644
--- a/src/box/engine_memtx.cc
+++ b/src/box/engine_memtx.cc
@@ -120,7 +120,7 @@ MemtxFactory::dropIndex(Index *index)
index->initIterator(it, ITER_ALL, NULL, 0);
struct tuple *tuple;
while ((tuple = it->next(it)))
- tuple_ref(tuple, -1);
+ tuple_unref(tuple);
}
void
diff --git a/src/box/engine_sophia.cc b/src/box/engine_sophia.cc
index 4231882856..71cb5e9823 100644
--- a/src/box/engine_sophia.cc
+++ b/src/box/engine_sophia.cc
@@ -217,6 +217,6 @@ SophiaFactory::txnFinish(struct txn *txn)
#if 0
struct txn_stmt *stmt = txn_stmt(txn);
if (stmt->new_tuple)
- tuple_ref(stmt->new_tuple, -1);
+ tuple_unref(stmt->new_tuple);
#endif
}
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 0999acf65e..0addfbc821 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -239,11 +239,9 @@ box.schema.space.drop = function(space_id)
local v = keys[i]
_index:delete{v[1], v[2]}
end
- local privs = _priv:select{}
+ local privs = _priv.index.object:select{'space', space_id}
for k, tuple in pairs(privs) do
- if tuple[3] == 'space' and tuple[4] == space_id then
- box.schema.user.revoke(tuple[2], tuple[5], tuple[3], tuple[4])
- end
+ box.schema.user.revoke(tuple[2], tuple[5], tuple[3], tuple[4])
end
if _space:delete{space_id} == nil then
box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id))
@@ -935,11 +933,9 @@ box.schema.func.drop = function(name)
local _func = box.space[box.schema.FUNC_ID]
local _priv = box.space[box.schema.PRIV_ID]
local fid = object_resolve('function', name)
- local privs = _priv:select{}
+ local privs = _priv.index.object:select{'function', fid}
for k, tuple in pairs(privs) do
- if tuple[3] == 'function' and tuple[4] == fid then
- box.schema.user.revoke(tuple[2], tuple[5], tuple[3], tuple[4])
- end
+ box.schema.user.revoke(tuple[2], tuple[5], tuple[3], tuple[4])
end
_func:delete{fid}
end
diff --git a/src/box/lua/tuple.cc b/src/box/lua/tuple.cc
index a5057c1bec..f0c204c4bb 100644
--- a/src/box/lua/tuple.cc
+++ b/src/box/lua/tuple.cc
@@ -106,7 +106,7 @@ static int
lbox_tuple_gc(struct lua_State *L)
{
struct tuple *tuple = lua_checktuple(L, 1);
- tuple_ref(tuple, -1);
+ tuple_unref(tuple);
return 0;
}
@@ -305,7 +305,7 @@ lbox_pushtuple(struct lua_State *L, struct tuple *tuple)
*ptr = tuple;
lua_pushcfunction(L, lbox_tuple_gc);
luaL_setcdatagc(L, -2);
- tuple_ref(tuple, 1);
+ tuple_ref(tuple);
} else {
return lua_pushnil(L);
}
diff --git a/src/box/lua/tuple.lua b/src/box/lua/tuple.lua
index 515c792f47..cd0991f6a5 100644
--- a/src/box/lua/tuple.lua
+++ b/src/box/lua/tuple.lua
@@ -16,8 +16,10 @@ struct tuple
char data[0];
} __attribute__((packed));
+int
+tuple_ref_nothrow(struct tuple *tuple);
void
-tuple_ref(struct tuple *tuple, int count);
+tuple_unref(struct tuple *tuple);
uint32_t
tuple_field_count(const struct tuple *tuple);
const char *
@@ -50,13 +52,16 @@ local builtin = ffi.C
local const_struct_tuple_ref_t = ffi.typeof('const struct tuple&')
local tuple_gc = function(tuple)
- builtin.tuple_ref(tuple, -1)
+ builtin.tuple_unref(tuple)
end
local tuple_bless = function(tuple)
- -- update in-place, do not spent time calling tuple_ref
+ -- tuple_ref(..) may throw to prevent reference counter to overflow,
+ -- which is not allowed in ffi call, so we'll use nothrow version
+ if (builtin.tuple_ref_nothrow(tuple) ~= 0) then
+ box.error();
+ end
local tuple2 = ffi.gc(ffi.cast(const_struct_tuple_ref_t, tuple), tuple_gc)
- tuple._refs = tuple._refs + 1
return tuple2
end
diff --git a/src/box/sophia_index.cc b/src/box/sophia_index.cc
index b50bf1c2c1..1b2ee0536d 100644
--- a/src/box/sophia_index.cc
+++ b/src/box/sophia_index.cc
@@ -128,7 +128,7 @@ sophia_gettuple(void *db, const char *key, size_t keysize,
void *value = sp_get(result, "value", &valuesize);
struct tuple *ret =
tuple_new(format, (char*)value, (char*)value + valuesize);
- tuple_ref(ret, 1);
+ tuple_ref(ret);
return ret;
}
@@ -197,7 +197,7 @@ SophiaIndex::random(uint32_t rnd) const
struct tuple *ret =
tuple_new(space->format, (char*)value,
(char*)value + valuesize);
- tuple_ref(ret, 1);
+ tuple_ref(ret);
return ret;
}
@@ -282,14 +282,14 @@ SophiaIndex::replace(struct tuple *old_tuple, struct tuple *new_tuple,
sophia_check_dup(key_def, old_tuple, dup_tuple, mode);
if (errcode) {
if (dup_tuple)
- tuple_ref(dup_tuple, -1);
+ tuple_unref(dup_tuple);
tnt_raise(ClientError, errcode, index_id(this));
}
void *o = sp_object(db);
if (o == NULL) {
if (dup_tuple)
- tuple_ref(dup_tuple, -1);
+ tuple_unref(dup_tuple);
tnt_raise(ClientError, ER_SOPHIA, sp_error(db));
}
sp_set(o, "key", key, keysize);
@@ -297,7 +297,7 @@ SophiaIndex::replace(struct tuple *old_tuple, struct tuple *new_tuple,
int rc = sp_set(db, o);
if (rc == -1) {
if (dup_tuple)
- tuple_ref(dup_tuple, -1);
+ tuple_unref(dup_tuple);
tnt_raise(ClientError, ER_SOPHIA, sp_error(db));
}
if (dup_tuple)
@@ -352,7 +352,7 @@ sophia_iterator_next(struct iterator *ptr)
const char *value = (const char*)sp_get(o, "value", &valuesize);
struct tuple *ret =
tuple_new(it->space->format, value, value + valuesize);
- tuple_ref(ret, 1);
+ tuple_ref(ret);
return ret;
}
diff --git a/src/box/tuple.cc b/src/box/tuple.cc
index 69d2dff189..4c656f84c8 100644
--- a/src/box/tuple.cc
+++ b/src/box/tuple.cc
@@ -284,19 +284,12 @@ tuple_delete(struct tuple *tuple)
}
/**
- * Add count to tuple's reference counter.
- * When the counter goes down to 0, the tuple is destroyed.
- *
- * @pre tuple->refs + count >= 0
+ * Throw and exception about tuple reference counter overflow.
*/
void
-tuple_ref(struct tuple *tuple, int count)
+tuple_ref_exception()
{
- assert(tuple->refs + count >= 0);
- tuple->refs += count;
-
- if (tuple->refs == 0)
- tuple_delete(tuple);
+ tnt_raise(ClientError, ER_TUPLE_REF_OVERFLOW);
}
const char *
diff --git a/src/box/tuple.h b/src/box/tuple.h
index 6e79795fee..959a75da26 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -32,6 +32,7 @@
#include "key_def.h" /* for enum field_type */
enum { FORMAT_ID_MAX = UINT16_MAX - 1, FORMAT_ID_NIL = UINT16_MAX };
+enum { FORMAT_REF_MAX = INT32_MAX, TUPLE_REF_MAX = UINT16_MAX };
struct tbuf;
@@ -123,6 +124,8 @@ static inline void
tuple_format_ref(struct tuple_format *format, int count)
{
assert(format->refs + count >= 0);
+ assert((uint64_t)format->refs + count <= FORMAT_REF_MAX);
+
format->refs += count;
if (format->refs == 0)
tuple_format_delete(format);
@@ -171,21 +174,73 @@ struct tuple *
tuple_new(struct tuple_format *format, const char *data, const char *end);
/**
- * Change tuple reference counter. If it has reached zero, free the tuple.
+ * Free the tuple.
+ * @pre tuple->refs == 0
+ */
+void
+tuple_delete(struct tuple *tuple);
+
+/**
+ * Throw and exception about tuple reference counter overflow.
+ */
+void
+tuple_ref_exception();
+
+/**
+ * Increment tuple reference counter.
+ * Throws if overflow detected.
*
* @pre tuple->refs + count >= 0
*/
-extern "C" void
-tuple_ref(struct tuple *tuple, int count);
+extern "C" inline void
+tuple_ref(struct tuple *tuple)
+{
+ if (tuple->refs + 1 > TUPLE_REF_MAX)
+ tuple_ref_exception();
-void
-tuple_delete(struct tuple *tuple);
+ tuple->refs++;
+}
+
+/**
+ * Increment tuple reference counter.
+ * Returns -1 if overflow detected, 0 otherwise
+ *
+ * @pre tuple->refs + count >= 0
+ */
+extern "C" inline int
+tuple_ref_nothrow(struct tuple *tuple)
+{
+ try {
+ tuple_ref(tuple);
+ } catch (Exception *e) {
+ return -1;
+ }
+ return 0;
+}
+
+/**
+ * Decrement tuple reference counter. If it has reached zero, free the tuple.
+ *
+ * @pre tuple->refs + count >= 0
+ */
+extern "C" inline void
+tuple_unref(struct tuple *tuple)
+{
+ assert(tuple->refs - 1 >= 0);
+
+ tuple->refs--;
+
+ if (tuple->refs == 0)
+ tuple_delete(tuple);
+}
/** Make tuple references exception-friendly in absence of @finally. */
struct TupleGuard {
struct tuple *tuple;
- TupleGuard(struct tuple *arg) :tuple(arg) {}
- ~TupleGuard() { if (tuple->refs == 0) tuple_delete(tuple); }
+ TupleGuard(struct tuple *arg) :tuple(arg) { tuple_ref(tuple); }
+ ~TupleGuard() { tuple_unref(tuple); }
+ TupleGuard(const TupleGuard&) = delete;
+ void operator=(const TupleGuard&) = delete;
};
/**
diff --git a/src/box/txn.cc b/src/box/txn.cc
index c350593c9d..180980e2c3 100644
--- a/src/box/txn.cc
+++ b/src/box/txn.cc
@@ -84,7 +84,7 @@ txn_replace(struct txn *txn, struct space *space,
stmt->old_tuple = space_replace(space, old_tuple, new_tuple, mode);
if (new_tuple) {
stmt->new_tuple = new_tuple;
- tuple_ref(stmt->new_tuple, 1);
+ tuple_ref(stmt->new_tuple);
}
stmt->space = space;
/**
@@ -225,7 +225,7 @@ txn_finish(struct txn *txn)
struct txn_stmt *stmt;
rlist_foreach_entry(stmt, &txn->stmts, next) {
if (stmt->old_tuple)
- tuple_ref(stmt->old_tuple, -1);
+ tuple_unref(stmt->old_tuple);
if (stmt->space)
stmt->space->engine->factory->txnFinish(txn);
}
@@ -254,7 +254,7 @@ txn_rollback_stmt()
space_replace(stmt->space, stmt->new_tuple,
stmt->old_tuple, DUP_INSERT);
if (stmt->new_tuple)
- tuple_ref(stmt->new_tuple, -1);
+ tuple_unref(stmt->new_tuple);
}
stmt->old_tuple = stmt->new_tuple = NULL;
stmt->space = NULL;
@@ -277,7 +277,7 @@ txn_rollback()
trigger_run(&txn->on_rollback, txn); /* must not throw. */
rlist_foreach_entry(stmt, &txn->stmts, next) {
if (stmt->new_tuple)
- tuple_ref(stmt->new_tuple, -1);
+ tuple_unref(stmt->new_tuple);
}
/* if (!txn->autocommit && txn->n_stmts && engine_no_yield(txn->engine)) */
trigger_clear(&txn->fiber_on_yield);
diff --git a/src/errcode.h b/src/errcode.h
index 355699e767..41436381a3 100644
--- a/src/errcode.h
+++ b/src/errcode.h
@@ -135,6 +135,7 @@ enum { TNT_ERRMSG_MAX = 512 };
/* 83 */_(ER_ROLE_EXISTS, 2, "Role '%s' already exists") \
/* 84 */_(ER_CREATE_ROLE, 2, "Failed to create role '%s': %s") \
/* 85 */_(ER_INDEX_EXISTS, 2, "Index '%s' already exists") \
+ /* 86 */_(ER_TUPLE_REF_OVERFLOW, 1, "Tuple reference counter is overflowed") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/ffisyms.cc b/src/ffisyms.cc
index 725dddb914..808d7af999 100644
--- a/src/ffisyms.cc
+++ b/src/ffisyms.cc
@@ -27,7 +27,8 @@ void *ffi_symbols[] = {
(void *) tuple_rewind,
(void *) tuple_seek,
(void *) tuple_next,
- (void *) tuple_ref,
+ (void *) tuple_ref_nothrow,
+ (void *) tuple_unref,
(void *) boxffi_index_len,
(void *) boxffi_index_random,
(void *) boxffi_index_iterator,
diff --git a/test/big/lua.result b/test/big/lua.result
index 2d7a81ec47..f460a36fb0 100644
--- a/test/big/lua.result
+++ b/test/big/lua.result
@@ -734,7 +734,7 @@ t:find(2, '2')
...
t:find(89, '2')
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:findall(4, '3')
---
diff --git a/test/box/misc.result b/test/box/misc.result
index f779861515..5a65842d9d 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -208,6 +208,7 @@ t;
- 'box.error.WAL_IO : 40'
- 'box.error.CREATE_USER : 43'
- 'box.error.CREATE_SPACE : 9'
+ - 'box.error.TUPLE_REF_OVERFLOW : 86'
- 'box.error.UNKNOWN_SCHEMA_OBJECT : 49'
- 'box.error.PROC_LUA : 32'
- 'box.error.CREATE_ROLE : 84'
diff --git a/test/box/select.result b/test/box/select.result
index 0d39be3522..9165a6e000 100644
--- a/test/box/select.result
+++ b/test/box/select.result
@@ -538,3 +538,32 @@ s:select(2)
s:drop()
---
...
+s = box.schema.create_space('select', { temporary = true })
+---
+...
+s:create_index('primary', { type = 'tree' })
+---
+...
+local a s:insert{0}
+---
+...
+lots_of_links = {}
+---
+...
+ref_count = 0
+---
+...
+while (true) do table.insert(lots_of_links, s:get{0}) ref_count = ref_count + 1 end
+---
+- error: Tuple reference counter is overflowed
+...
+ref_count
+---
+- 65534
+...
+lots_of_links = {}
+---
+...
+s:drop()
+---
+...
diff --git a/test/box/select.test.lua b/test/box/select.test.lua
index c09a1f7f16..385a3c295b 100644
--- a/test/box/select.test.lua
+++ b/test/box/select.test.lua
@@ -76,3 +76,13 @@ s.index[0]:select(1, { iterator = 'GE', offset = 10, limit = 2 })
s:select(2)
s:drop()
+
+s = box.schema.create_space('select', { temporary = true })
+s:create_index('primary', { type = 'tree' })
+local a s:insert{0}
+lots_of_links = {}
+ref_count = 0
+while (true) do table.insert(lots_of_links, s:get{0}) ref_count = ref_count + 1 end
+ref_count
+lots_of_links = {}
+s:drop()
diff --git a/test/box/tuple.result b/test/box/tuple.result
index 3cf34faca9..975be871b5 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -377,15 +377,15 @@ t:next(3)
...
t:next(4)
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:next(-1)
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:next("fdsaf")
---
-- error: '[string "-- tuple.lua (internal file)..."]:69: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:74: error: invalid key to ''next'''
...
box.tuple.new({'x', 'y', 'z'}):next()
---
@@ -397,7 +397,7 @@ t=space:insert{1953719668}
...
t:next(1684234849)
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:next(1)
---
@@ -553,7 +553,7 @@ r = {}
...
for _state, val in t:pairs(10) do table.insert(r, val) end
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
r
---
@@ -639,19 +639,19 @@ t:findall(1, 'xxxxx')
...
t:find(100, 'a')
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:findall(100, 'a')
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:find(100, 'xxxxx')
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
t:findall(100, 'xxxxx')
---
-- error: '[string "-- tuple.lua (internal file)..."]:86: error: invalid key to ''next'''
+- error: '[string "-- tuple.lua (internal file)..."]:91: error: invalid key to ''next'''
...
---
-- Lua type coercion
@@ -745,12 +745,12 @@ t = box.tuple.new({'a', 'b', 'c', 'd', 'e'})
...
t:update()
---
-- error: '[string "-- tuple.lua (internal file)..."]:152: Usage: tuple:update({ {
+- error: '[string "-- tuple.lua (internal file)..."]:157: Usage: tuple:update({ {
op, field, arg}+ })'
...
t:update(10)
---
-- error: '[string "-- tuple.lua (internal file)..."]:152: Usage: tuple:update({ {
+- error: '[string "-- tuple.lua (internal file)..."]:157: Usage: tuple:update({ {
op, field, arg}+ })'
...
t:update({})
--
GitLab