Commit fa787025 authored by Kevin Modzelewski's avatar Kevin Modzelewski

Fix a list GC bug

The presence of an allocated "elts" pointer is determined by capacity>0,
but the GC handler was incorrectly only choosing to visit it if size>0.
Not all code paths free the elts array if size goes down to zero, so
it could happen that capacity>0 and size=0, and then the elts array would
not be tracked even though it was expected to be.

Added a test that illustrates this.
parent 4b882187
...@@ -28,6 +28,9 @@ ...@@ -28,6 +28,9 @@
#include "valgrind.h" #include "valgrind.h"
#endif #endif
//#undef VERBOSITY
//#define VERBOSITY(x) 2
namespace pyston { namespace pyston {
namespace gc { namespace gc {
......
...@@ -26,6 +26,9 @@ ...@@ -26,6 +26,9 @@
#include "valgrind.h" #include "valgrind.h"
#endif #endif
//#undef VERBOSITY
//#define VERBOSITY(x) 2
namespace pyston { namespace pyston {
namespace gc { namespace gc {
......
...@@ -65,7 +65,6 @@ void BoxedList::shrink() { ...@@ -65,7 +65,6 @@ void BoxedList::shrink() {
capacity = new_capacity; capacity = new_capacity;
} else if (size == 0) { } else if (size == 0) {
rt_free(elts); rt_free(elts);
elts = NULL;
capacity = 0; capacity = 0;
} }
} }
......
...@@ -147,10 +147,12 @@ extern "C" void listGCHandler(GCVisitor* v, void* p) { ...@@ -147,10 +147,12 @@ extern "C" void listGCHandler(GCVisitor* v, void* p) {
BoxedList* l = (BoxedList*)p; BoxedList* l = (BoxedList*)p;
int size = l->size; int size = l->size;
if (size) { int capacity = l->capacity;
assert(capacity >= size);
if (capacity)
v->visit(l->elts); v->visit(l->elts);
if (size)
v->visitRange((void**)&l->elts->elts[0], (void**)&l->elts->elts[size]); v->visitRange((void**)&l->elts->elts[0], (void**)&l->elts->elts[size]);
}
static StatCounter sc("gc_listelts_visited"); static StatCounter sc("gc_listelts_visited");
sc.log(size); sc.log(size);
......
# Regression test:
# - create some lists
# - pop from them to make them empty, but still have capacity allocated
# - do some stuff to try to force a GC collection
# -- the old bug was that the element array would get freed because size=0 even though capacity>0
# - add more stuff to the array to try to get it to realloc more space
# -- crash
l = []
for i in xrange(100):
l2 = [0]
l.append(l2)
l2.pop()
for l2 in l:
l2 += range(100)
# allocate some data to try to force a collection:
# TODO shouldn't be too hard to add support for gc.collect()
range(10000)
Markdown is supported
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