Skip to content

Commit

Permalink
Do not use rb_str_tmp_frozen_no_embed_acquire with MMTk
Browse files Browse the repository at this point in the history
It was intended to work around https://bugs.ruby-lang.org/issues/20169,
but it is unnecessary for MMTk.
  • Loading branch information
wks committed Sep 14, 2024
1 parent b9e24e8 commit f38ff71
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions string.c
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,20 @@ rb_str_tmp_frozen_acquire(VALUE orig)
VALUE
rb_str_tmp_frozen_no_embed_acquire(VALUE orig)
{
WHEN_USING_MMTK({
// The default GC may lock (mprotect with PROT_NONE) pages of GC memory during compaction.
// If that happens while a pointer to the string payload in an embedded string is used for
// IO, with GVL released, the OS or external native programs may read or write protected
// pages, causing segmentation fault. The intention of `rb_str_tmp_frozen_no_embed_acquire`
// is working around this problem by forcing the given `orig` string to be allocated in the
// malloc heap.
//
// When using MMTk, this doesn't make sense because MMTk never protect pages of live
// objects, and we allocate non-embedded string buffers as `imemo:mmtk_strbuf` in the MMTk
// heap. So we simply call `rb_str_tmp_frozen_acquire` instead.
return rb_str_tmp_frozen_acquire(orig);
})

if (OBJ_FROZEN_RAW(orig) && !STR_EMBED_P(orig) && !rb_str_reembeddable_p(orig)) return orig;
if (STR_SHARED_P(orig) && !STR_EMBED_P(RSTRING(orig)->as.heap.aux.shared)) return rb_str_tmp_frozen_acquire(orig);

Expand All @@ -1762,13 +1776,6 @@ rb_str_tmp_frozen_no_embed_acquire(VALUE orig)
RSTRING(str)->as.heap.ptr = RSTRING(orig)->as.heap.ptr;
RBASIC(str)->flags |= RBASIC(orig)->flags & STR_NOFREE;
RBASIC(orig)->flags &= ~STR_NOFREE;

WHEN_USING_MMTK({
// Note: The root may be no-free.
VALUE strbuf = rb_mmtk_str_get_strbuf_nullable(orig);
rb_mmtk_str_set_strbuf(str, strbuf);
})

STR_SET_SHARED(orig, str);
}

Expand Down

0 comments on commit f38ff71

Please sign in to comment.