Skip to content
Snippets Groups Projects
Commit 52a25610 authored by Konstantin Osipov's avatar Konstantin Osipov
Browse files

A fix and a test case for Bug#1051006 

A fix and a test case for Bug#1051006
"Tree iterators return garbage if an index is modified between calls"

Mark in a deleted node in sptree.h that it's been  put into the
garbage heap. When iterting over a garbage collected node, skip it,
and go up the stack until we find the first valid node.

This breaks the "sorted" quality of tree iterators in case there
are modifications between invocations of an iterator:
it is possible that a node is deleted and recycled, and we don't see
it in the iterator. When we go up the stack, we can jump to a different
part of the range than the one the recycled node belongs to.
. With this fix, it is also possible, that the iteration goes more
than once over entire tree range. But it's a good enough quick fix for a
crashing expire loop, which uses the tree iterator over the primary key to
scan the entire range and deletes expired keys on the go (additionally,
deletions may occur between invocations of the expire loop).
parent deb58f46
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
mod/box/box_lua.m
+ 3
− 1
View file @ 52a25610
......@@ -81,7 +81,9 @@ static const char *tuplelib_name = "box.tuple";
static inline struct tuple *
lua_checktuple(struct lua_State *L, int narg)
{
return *(void **) luaL_checkudata(L, narg, tuplelib_name);
struct tuple *t = *(void **) luaL_checkudata(L, narg, tuplelib_name);
assert(t->refs);
return t;
}
struct tuple *
......
test/box_big/tree_pk_multipart.result
+ 87
− 0
View file @ 52a25610
......@@ -244,3 +244,90 @@ lua box.space[9]:len()
---
- 0
...
A test case for Bug#1051006 Tree iterators return garbage
if an index is modified between calls
lua box.space[16]:insert('a', 'a', 'a')
---
- 'a': {'a', 'a'}
...
lua box.space[16]:insert('d', 'd', 'd')
---
- 'd': {'d', 'd'}
...
lua box.space[16]:insert('e', 'e', 'e')
---
- 'e': {'e', 'e'}
...
lua box.space[16]:insert('b', 'b', 'b')
---
- 'b': {'b', 'b'}
...
lua box.space[16]:insert('c', 'c', 'c')
---
- 'c': {'c', 'c'}
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
'a': {'a', 'a'}
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
'b': {'b', 'b'}
...
lua box.space[16]:truncate()
---
...
lua print(v)
---
'b': {'b', 'b'}
...
lua collectgarbage('collect')
---
- 0
...
lua print(v)
---
'b': {'b', 'b'}
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
nil
...
lua collectgarbage('collect')
---
- 0
...
lua print(v)
---
nil
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
nil
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
nil
...
lua k,v = box.space[16].index[1]:next(k)
---
...
lua print(v)
---
nil
...
test/box_big/tree_pk_multipart.test
+ 27
− 0
View file @ 52a25610
......@@ -112,3 +112,30 @@ exec admin "lua box.update(9, {'The Wolf!', 'Vincent', 1, 'Come again?'}, '=p',
exec admin "lua box.space[9]:len()"
exec admin "lua box.space[9]:truncate()"
exec admin "lua box.space[9]:len()"
print """ A test case for Bug#1051006 Tree iterators return garbage
if an index is modified between calls"""
exec admin "lua box.space[16]:insert('a', 'a', 'a')"
exec admin "lua box.space[16]:insert('d', 'd', 'd')"
exec admin "lua box.space[16]:insert('e', 'e', 'e')"
exec admin "lua box.space[16]:insert('b', 'b', 'b')"
exec admin "lua box.space[16]:insert('c', 'c', 'c')"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
exec admin "lua box.space[16]:truncate()"
exec admin "lua print(v)"
exec admin "lua collectgarbage('collect')"
exec admin "lua print(v)"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
exec admin "lua collectgarbage('collect')"
exec admin "lua print(v)"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
exec admin "lua k,v = box.space[16].index[1]:next(k)"
exec admin "lua print(v)"
third_party/sptree.h
+ 55
− 28
View file @ 52a25610
......@@ -290,6 +290,11 @@ sptree_##name##_balance(sptree_##name *t, spnode_t node, spnode_t size) {
\
z = _GET_SPNODE_LEFT(fake); \
_SET_SPNODE_LEFT(fake, t->garbage_head); \
/* \
* Loop back on the right link indicates that the node \
* is in the garbage list. \
*/ \
_SET_SPNODE_RIGHT(fake, fake); \
t->garbage_head = fake; \
return z; \
} \
......@@ -430,6 +435,11 @@ sptree_##name##_delete(sptree_##name *t, void *k) {
} \
\
_SET_SPNODE_LEFT(node, t->garbage_head); \
/* \
* Loop back on the right link indicates that the node \
* is in the garbage list. \
*/ \
_SET_SPNODE_RIGHT(node, node); \
t->garbage_head = node; \
\
break; \
......@@ -645,50 +655,67 @@ sptree_##name##_iterator_free(sptree_##name##_iterator *i) {
free(i); \
} \
\
/** Nodes in the garbage list have a loop on their right link. */ \
static inline bool \
sptree_##name##_node_is_deleted(sptree_##name *t, spnode_t node) { \
\
return _GET_SPNODE_RIGHT(node) == node; \
} \
\
/** \
* Get the last node on the iterator stack, check \
* if the node is not deleted. \
*/ \
static inline spnode_t \
sptree_##name##_iterator_next_node(sptree_##name##_iterator *i) { \
\
while (i->level >= 0) { \
spnode_t return_node = i->stack[i->level--]; \
if (! sptree_##name##_node_is_deleted(i->t, return_node)) \
return return_node; \
} \
return SPNIL; \
} \
\
static inline void* \
sptree_##name##_iterator_next(sptree_##name##_iterator *i) { \
sptree_##name *t; \
spnode_t node, returnNode = SPNIL; \
\
if (i == NULL) return NULL; \
\
t = i->t; \
if ( i->level >= 0 ) { \
returnNode = i->stack[i->level]; \
sptree_##name *t = i->t; \
spnode_t returnNode = sptree_##name##_iterator_next_node(i); \
\
node = _GET_SPNODE_RIGHT( i->stack[i->level] ); \
i->level--; \
while( node != SPNIL ) { \
i->level++; \
i->stack[i->level] = node; \
node = _GET_SPNODE_LEFT( i->stack[i->level] ); \
} \
if (returnNode == SPNIL) return NULL; \
\
spnode_t node = _GET_SPNODE_RIGHT(returnNode); \
while (node != SPNIL) { \
i->level++; \
i->stack[i->level] = node; \
node = _GET_SPNODE_LEFT(i->stack[i->level]); \
} \
\
return (returnNode == SPNIL) ? NULL : ITHELEM(t, returnNode); \
return ITHELEM(t, returnNode); \
} \
\
static inline void* \
sptree_##name##_iterator_reverse_next(sptree_##name##_iterator *i) { \
sptree_##name *t; \
spnode_t node, returnNode = SPNIL; \
\
if (i == NULL) return NULL; \
\
t = i->t; \
if ( i->level >= 0 ) { \
returnNode = i->stack[i->level]; \
sptree_##name *t = i->t; \
spnode_t returnNode = sptree_##name##_iterator_next_node(i); \
\
node = _GET_SPNODE_LEFT( i->stack[i->level] ); \
i->level--; \
while( node != SPNIL ) { \
i->level++; \
i->stack[i->level] = node; \
node = _GET_SPNODE_RIGHT( i->stack[i->level] ); \
} \
} \
if (returnNode == SPNIL) return NULL; \
\
return (returnNode == SPNIL) ? NULL : ITHELEM(t, returnNode); \
spnode_t node = _GET_SPNODE_LEFT(returnNode); \
while (node != SPNIL) { \
i->level++; \
i->stack[i->level] = node; \
node = _GET_SPNODE_RIGHT(i->stack[i->level]); \
} \
return ITHELEM(t, returnNode); \
}
/*
* vim: ts=4 sts=4 et
*/
#endif
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment