Skip to content

Commit b82fefc

Browse files
committed
Make C extension Ractor-safe (xmalloc rmem pages, declare rb_ext_ractor_safe)
The page-recycling slab was a process-global msgpack_rmem_t mutated through an unsynchronized bitmask, so parallel Ractors (or threads) raced on it. Drop the slab and serve rmem pages straight from xmalloc/xfree (jemalloc and friends recycle these fine), then declare rb_ext_ractor_safe(true). The declaration is guarded by HAVE_RB_EXT_RACTOR_SAFE so the extension still builds on Ruby < 3.0. Add spec/cruby/ractor_spec.rb covering pack/unpack via Factory and Packer/Unpacker from non-main Ractors, including a concurrent multi-Ractor stress over the page-allocation path.
1 parent 09c914d commit b82fefc

7 files changed

Lines changed: 78 additions & 186 deletions

File tree

ext/msgpack/buffer.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,17 @@ int msgpack_rb_encindex_ascii8bit;
2525

2626
ID s_uminus;
2727

28-
static msgpack_rmem_t s_rmem;
29-
3028
void msgpack_buffer_static_init(void)
3129
{
3230
s_uminus = rb_intern("-@");
3331

3432
msgpack_rb_encindex_utf8 = rb_utf8_encindex();
3533
msgpack_rb_encindex_usascii = rb_usascii_encindex();
3634
msgpack_rb_encindex_ascii8bit = rb_ascii8bit_encindex();
37-
38-
msgpack_rmem_init(&s_rmem);
3935
}
4036

4137
void msgpack_buffer_static_destroy(void)
4238
{
43-
msgpack_rmem_destroy(&s_rmem);
4439
}
4540

4641
void msgpack_buffer_init(msgpack_buffer_t* b)
@@ -59,9 +54,7 @@ static void _msgpack_buffer_chunk_destroy(msgpack_buffer_chunk_t* c)
5954
{
6055
if(c->mem != NULL) {
6156
if(c->rmem) {
62-
if(!msgpack_rmem_free(&s_rmem, c->mem)) {
63-
rb_bug("Failed to free an rmem pointer, memory leak?");
64-
}
57+
msgpack_rmem_free(c->mem);
6558
} else {
6659
xfree(c->mem);
6760
}
@@ -354,7 +347,7 @@ static inline void* _msgpack_buffer_chunk_malloc(
354347
if((size_t)(b->rmem_end - b->rmem_last) < required_size) {
355348
/* alloc new rmem page */
356349
*allocated_size = MSGPACK_RMEM_PAGE_SIZE;
357-
char* buffer = msgpack_rmem_alloc(&s_rmem);
350+
char* buffer = msgpack_rmem_alloc();
358351
c->mem = buffer;
359352

360353
/* update rmem owner */

ext/msgpack/extconf.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
require 'mkmf'
22

33
have_func("rb_enc_interned_str", "ruby.h") # Ruby 3.0+
4+
have_func("rb_ext_ractor_safe", "ruby.h") # Ruby 3.0+
45
have_func("rb_hash_new_capa", "ruby.h") # Ruby 3.2+
56
have_func("rb_proc_call_with_block", "ruby.h") # CRuby (TruffleRuby doesn't have it)
67
have_func("rb_gc_mark_locations", "ruby.h") # Missing on TruffleRuby

ext/msgpack/rbinit.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@
2424

2525
RUBY_FUNC_EXPORTED void Init_msgpack(void)
2626
{
27+
/* No process-global mutable state, so packing/unpacking is safe per-Ractor.
28+
* rb_ext_ractor_safe is Ruby 3.0+, so guard for older supported Rubies. */
29+
#ifdef HAVE_RB_EXT_RACTOR_SAFE
30+
rb_ext_ractor_safe(true);
31+
#endif
32+
2733
VALUE mMessagePack = rb_define_module("MessagePack");
2834

2935
MessagePack_Buffer_module_init(mMessagePack);

ext/msgpack/rmem.c

Lines changed: 0 additions & 93 deletions
This file was deleted.

ext/msgpack/rmem.h

Lines changed: 6 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -19,91 +19,22 @@
1919
#define MSGPACK_RUBY_RMEM_H__
2020

2121
#include "compat.h"
22-
#include "sysdep.h"
2322

2423
#ifndef MSGPACK_RMEM_PAGE_SIZE
2524
#define MSGPACK_RMEM_PAGE_SIZE (4*1024)
2625
#endif
2726

28-
struct msgpack_rmem_t;
29-
typedef struct msgpack_rmem_t msgpack_rmem_t;
30-
31-
struct msgpack_rmem_chunk_t;
32-
typedef struct msgpack_rmem_chunk_t msgpack_rmem_chunk_t;
33-
34-
/*
35-
* a chunk contains 32 pages.
36-
* size of each buffer is MSGPACK_RMEM_PAGE_SIZE bytes.
37-
*/
38-
struct msgpack_rmem_chunk_t {
39-
unsigned int mask;
40-
char* pages;
41-
};
42-
43-
struct msgpack_rmem_t {
44-
msgpack_rmem_chunk_t head;
45-
msgpack_rmem_chunk_t* array_first;
46-
msgpack_rmem_chunk_t* array_last;
47-
msgpack_rmem_chunk_t* array_end;
48-
};
49-
50-
/* assert MSGPACK_RMEM_PAGE_SIZE % sysconf(_SC_PAGE_SIZE) == 0 */
51-
void msgpack_rmem_init(msgpack_rmem_t* pm);
52-
53-
void msgpack_rmem_destroy(msgpack_rmem_t* pm);
54-
55-
void* _msgpack_rmem_alloc2(msgpack_rmem_t* pm);
56-
57-
#define _msgpack_rmem_chunk_available(c) ((c)->mask != 0)
58-
59-
static inline void* _msgpack_rmem_chunk_alloc(msgpack_rmem_chunk_t* c)
27+
/* Fixed-size scratch pages for the buffer and unpacker stack, served straight
28+
* from xmalloc/xfree so they can be allocated and freed from any Ractor. */
29+
static inline void* msgpack_rmem_alloc(void)
6030
{
61-
_msgpack_bsp32(pos, c->mask);
62-
(c)->mask &= ~(1 << pos);
63-
return ((char*)(c)->pages) + (pos * (MSGPACK_RMEM_PAGE_SIZE));
31+
return xmalloc(MSGPACK_RMEM_PAGE_SIZE);
6432
}
6533

66-
static inline bool _msgpack_rmem_chunk_try_free(msgpack_rmem_chunk_t* c, void* mem)
34+
static inline void msgpack_rmem_free(void* mem)
6735
{
68-
ptrdiff_t pdiff = ((char*)(mem)) - ((char*)(c)->pages);
69-
if(0 <= pdiff && pdiff < MSGPACK_RMEM_PAGE_SIZE * 32) {
70-
size_t pos = pdiff / MSGPACK_RMEM_PAGE_SIZE;
71-
(c)->mask |= (1 << pos);
72-
return true;
73-
}
74-
return false;
36+
xfree(mem);
7537
}
7638

77-
static inline void* msgpack_rmem_alloc(msgpack_rmem_t* pm)
78-
{
79-
if(_msgpack_rmem_chunk_available(&pm->head)) {
80-
return _msgpack_rmem_chunk_alloc(&pm->head);
81-
}
82-
return _msgpack_rmem_alloc2(pm);
83-
}
84-
85-
void _msgpack_rmem_chunk_free(msgpack_rmem_t* pm, msgpack_rmem_chunk_t* c);
86-
87-
static inline bool msgpack_rmem_free(msgpack_rmem_t* pm, void* mem)
88-
{
89-
if(_msgpack_rmem_chunk_try_free(&pm->head, mem)) {
90-
return true;
91-
}
92-
93-
/* search from last */
94-
msgpack_rmem_chunk_t* c = pm->array_last - 1;
95-
msgpack_rmem_chunk_t* before_first = pm->array_first - 1;
96-
for(; c != before_first; c--) {
97-
if(_msgpack_rmem_chunk_try_free(c, mem)) {
98-
if(c != pm->array_first && c->mask == 0xffffffff) {
99-
_msgpack_rmem_chunk_free(pm, c);
100-
}
101-
return true;
102-
}
103-
}
104-
return false;
105-
}
106-
107-
10839
#endif
10940

ext/msgpack/unpacker.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ static int RAW_TYPE_STRING = 256;
6464
static int RAW_TYPE_BINARY = 257;
6565
static int16_t INITIAL_BUFFER_CAPACITY_MAX = SHRT_MAX;
6666

67-
static msgpack_rmem_t s_stack_rmem;
68-
6967
#if !defined(HAVE_RB_HASH_NEW_CAPA)
7068
static inline VALUE rb_hash_new_capa_inline(long capa)
7169
{
@@ -82,21 +80,18 @@ static inline int16_t initial_buffer_size(long size)
8280
void msgpack_unpacker_static_init(void)
8381
{
8482
assert(sizeof(msgpack_unpacker_stack_entry_t) * MSGPACK_UNPACKER_STACK_CAPACITY <= MSGPACK_RMEM_PAGE_SIZE);
85-
86-
msgpack_rmem_init(&s_stack_rmem);
8783
}
8884

8985
void msgpack_unpacker_static_destroy(void)
9086
{
91-
msgpack_rmem_destroy(&s_stack_rmem);
9287
}
9388

9489
#define HEAD_BYTE_REQUIRED 0xc1
9590

9691
static inline bool _msgpack_unpacker_stack_init(msgpack_unpacker_stack_t *stack) {
9792
if (!stack->data) {
9893
stack->capacity = MSGPACK_UNPACKER_STACK_CAPACITY;
99-
stack->data = msgpack_rmem_alloc(&s_stack_rmem);
94+
stack->data = msgpack_rmem_alloc();
10095
stack->depth = 0;
10196
return true;
10297
}
@@ -105,9 +100,7 @@ static inline bool _msgpack_unpacker_stack_init(msgpack_unpacker_stack_t *stack)
105100

106101
static inline void _msgpack_unpacker_free_stack(msgpack_unpacker_stack_t* stack) {
107102
if (stack->data) {
108-
if (!msgpack_rmem_free(&s_stack_rmem, stack->data)) {
109-
rb_bug("Failed to free an rmem pointer, memory leak?");
110-
}
103+
msgpack_rmem_free(stack->data);
111104
stack->data = NULL;
112105
stack->depth = 0;
113106
}

spec/cruby/ractor_spec.rb

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
require 'spec_helper'
2+
3+
ractor_supported = defined?(Ractor) && RUBY_ENGINE == 'ruby'
4+
5+
describe 'Ractor safety', skip: (ractor_supported ? false : 'Ractor not supported on this Ruby') do
6+
def ractor_value(ractor)
7+
# Ractor#value replaced #take in newer rubies; support both.
8+
ractor.respond_to?(:value) ? ractor.value : ractor.take
9+
end
10+
11+
# Ruby prints a one-time "Ractor API is experimental" warning to stderr. Quiet
12+
# it for this group, and restore afterwards so we don't suppress the warning
13+
# for unrelated specs sharing the process.
14+
before(:all) do
15+
@experimental_warning = Warning[:experimental]
16+
Warning[:experimental] = false
17+
end
18+
19+
after(:all) do
20+
Warning[:experimental] = @experimental_warning
21+
end
22+
23+
it 'round-trips via a Factory inside a non-main Ractor' do
24+
result = ractor_value(Ractor.new do
25+
factory = MessagePack::Factory.new
26+
factory.load(factory.dump([1, "two", 3.0, nil, true, {"k" => "v"}]))
27+
end)
28+
expect(result).to eq([1, "two", 3.0, nil, true, {"k" => "v"}])
29+
end
30+
31+
it 'round-trips via a Packer and Unpacker inside a non-main Ractor' do
32+
result = ractor_value(Ractor.new do
33+
packed = MessagePack::Packer.new.write({"x" => [1, 2, 3]}).to_s
34+
MessagePack::Unpacker.new.feed(packed).read
35+
end)
36+
expect(result).to eq({"x" => [1, 2, 3]})
37+
end
38+
39+
it 'packs and unpacks concurrently across many Ractors without corruption' do
40+
ractor_count = 8
41+
42+
ractors = ractor_count.times.map do |n|
43+
Ractor.new(n) do |seed|
44+
obj = {
45+
"seed" => seed,
46+
"nums" => (0..20).to_a,
47+
"str" => "x" * 100,
48+
"nested" => {"deep" => [seed] * 10},
49+
}
50+
ok = true
51+
2_000.times do
52+
packed = MessagePack::Packer.new.write(obj).to_s
53+
ok &&= MessagePack::Unpacker.new.feed(packed).read == obj
54+
end
55+
ok
56+
end
57+
end
58+
59+
expect(ractors.map { |r| ractor_value(r) }).to all(be true)
60+
end
61+
end

0 commit comments

Comments
 (0)