Skip to content

Commit

Permalink
Fix bugs in the handling of objects with finalizers. This fixes 100% …
Browse files Browse the repository at this point in the history
…CPU infinite loop bugs that some people are experiencing.
  • Loading branch information
FooBarWidget committed Apr 3, 2009
1 parent bdecfad commit 63247bf
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 13 deletions.
9 changes: 5 additions & 4 deletions gc.c
Expand Up @@ -563,7 +563,6 @@ rb_newobj()
obj = (VALUE)freelist;
freelist = freelist->as.free.next;
MEMZERO((void*)obj, RVALUE, 1);
RANY(obj)->as.free.flags = FL_ALLOCATED;
#ifdef GC_DEBUG
RANY(obj)->file = ruby_sourcefile;
RANY(obj)->line = ruby_sourceline;
Expand Down Expand Up @@ -757,7 +756,7 @@ gc_mark_all()
p = heap->slot; pend = heap->slotlimit;
while (p < pend) {
if (rb_mark_table_heap_contains(heap, p) &&
(p->as.basic.flags != FL_ALLOCATED)) {
!(p->as.basic.flags & FL_DEFER_FINALIZE)) {
gc_mark_children((VALUE)p, 0);
}
p++;
Expand Down Expand Up @@ -1194,7 +1193,7 @@ finalize_list(p)
* The latter is to prevent the unnecessary marking of memory pages as dirty,
* which can destroy copy-on-write semantics.
*/
if (!FL_TEST(p, FL_SINGLETON) && p->as.free.flags != 0) {
if (!FL_TEST(p, FL_SINGLETON)) {
p->as.free.flags = 0;
p->as.free.next = freelist;
rb_mark_table_remove(p);
Expand Down Expand Up @@ -1283,6 +1282,7 @@ gc_sweep()
}
if (need_call_final && FL_TEST(p, FL_FINALIZE)) {
/* This object has a finalizer, so don't free it right now, but do it later. */
p->as.free.flags = FL_DEFER_FINALIZE;
rb_mark_table_heap_add(heap, p); /* remain marked */
p->as.free.next = final_list;
final_list = p;
Expand All @@ -1299,7 +1299,7 @@ gc_sweep()
}
n++;
}
else if (RBASIC(p)->flags == FL_ALLOCATED) {
else if (RBASIC(p)->flags & FL_DEFER_FINALIZE) {
/* objects to be finalized */
/* do nothing remain marked */
}
Expand All @@ -1313,6 +1313,7 @@ gc_sweep()
RVALUE *pp;

heaps[i].limit = 0;
heaps[i].slotlimit = heaps[i].slot;
for (pp = final_list; pp != final; pp = pp->as.free.next) {
pp->as.free.flags |= FL_SINGLETON; /* freeing page mark */
}
Expand Down
9 changes: 1 addition & 8 deletions marktable.c
Expand Up @@ -95,14 +95,7 @@ rb_bf_mark_table_prepare()
static void
rb_bf_mark_table_finalize()
{
int i;

if (pointer_set_get_size(mark_table) > 0) {
pointer_set_reset(mark_table);
}
for (i = 0; i < heaps_used; i++) {
memset(heaps[i].marks, 0, heaps[i].marks_size * sizeof(int));
}
/* Do nothing. */
}

static inline void
Expand Down
2 changes: 1 addition & 1 deletion ruby.h
Expand Up @@ -441,7 +441,7 @@ struct RBignum {
#define RFILE(obj) (R_CAST(RFile)(obj))

#define FL_SINGLETON FL_USER0
#define FL_ALLOCATED (1<<6)
#define FL_DEFER_FINALIZE (1<<6)
#define FL_FINALIZE (1<<7)
#define FL_MARK (1<<11)
#define FL_TAINT (1<<8)
Expand Down

2 comments on commit 63247bf

@rdp
Copy link

@rdp rdp commented on 63247bf Apr 6, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this something that should be pushed upstream?

@FooBarWidget
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this is our own bug.

Please sign in to comment.