Skip to content

Commit e3e8725

Browse files
committed
Fix issue where a new GC could be started during a GC
[Bug #21548] In lazy sweeping, if we need to allocate an object in a heap where we weren't able to free any slots, but we also either have empty pages or could allocate new pages, then we want to preemptively claim a page because it's possible that sweeping another heap will call gc_sweep_finish_heap, which may use up all of the empty/allocatable pages. If other heaps are not finished sweeping then we do not finish this GC and we will end up triggering a new GC cycle during this GC phase.
1 parent 34cca18 commit e3e8725

File tree

1 file changed

+25
-5
lines changed

1 file changed

+25
-5
lines changed

gc/default/default.c

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,6 +2052,9 @@ heap_prepare(rb_objspace_t *objspace, rb_heap_t *heap)
20522052
/* If we still don't have a free page and not allowed to create a new page,
20532053
* we should start a new GC cycle. */
20542054
if (heap->free_pages == NULL) {
2055+
GC_ASSERT(objspace->empty_pages_count == 0);
2056+
GC_ASSERT(objspace->heap_pages.allocatable_slots == 0);
2057+
20552058
if (gc_start(objspace, GPR_FLAG_NEWOBJ) == FALSE) {
20562059
rb_memerror();
20572060
}
@@ -3930,11 +3933,28 @@ gc_sweep_continue(rb_objspace_t *objspace, rb_heap_t *sweep_heap)
39303933
if (gc_sweep_step(objspace, heap)) {
39313934
GC_ASSERT(heap->free_pages != NULL);
39323935
}
3933-
else if (heap == sweep_heap && objspace->empty_pages_count == 0 && objspace->heap_pages.allocatable_slots == 0) {
3934-
/* Not allowed to create a new page so finish sweeping. */
3935-
gc_sweep_rest(objspace);
3936-
GC_ASSERT(gc_mode(objspace) == gc_mode_none);
3937-
break;
3936+
else if (heap == sweep_heap) {
3937+
if (objspace->empty_pages_count > 0 || objspace->heap_pages.allocatable_slots > 0) {
3938+
/* [Bug #21548]
3939+
*
3940+
* If this heap is the heap we want to sweep, but we weren't able
3941+
* to free any slots, but we also either have empty pages or could
3942+
* allocate new pages, then we want to preemptively claim a page
3943+
* because it's possible that sweeping another heap will call
3944+
* gc_sweep_finish_heap, which may use up all of the
3945+
* empty/allocatable pages. If other heaps are not finished sweeping
3946+
* then we do not finish this GC and we will end up triggering a new
3947+
* GC cycle during this GC phase. */
3948+
heap_page_allocate_and_initialize(objspace, heap);
3949+
3950+
GC_ASSERT(heap->free_pages != NULL);
3951+
}
3952+
else {
3953+
/* Not allowed to create a new page so finish sweeping. */
3954+
gc_sweep_rest(objspace);
3955+
GC_ASSERT(gc_mode(objspace) == gc_mode_none);
3956+
break;
3957+
}
39383958
}
39393959
}
39403960

0 commit comments

Comments
 (0)