Skip to content

Commit f40067b

Browse files
committed
Fix rb_global_variable() for Float and bignum values during the Init_ function
* We need to store the ValueWrapper and not the value/object itself in GC_REGISTERED_ADDRESSES, see #3478 (comment) * Fixes #3478
1 parent e7eb8ea commit f40067b

File tree

5 files changed

+39
-16
lines changed

5 files changed

+39
-16
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ Bug fixes:
77
* Add missing thread-safe objects write barriers for `TruffleRuby::ConcurrentMap` (#3179, @eregon).
88
* Fix repeated calling of methods `Dir#{each,each_child,children}` (#3464, @andrykonchin).
99
* Fix `IO#{wait,wait_readable,wait_writable}` methods and switch the current thread into a sleep state (@andrykonchin).
10+
* Fix `rb_global_variable()` for `Float` and bignum values during the `Init_` function (#3478, @eregon).
1011

1112
Compatibility:
1213
* Move `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core library (@andrykonchin).

lib/cext/ABI_check.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
14
1+
15

lib/truffle/truffle/cext.rb

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,15 @@ def init_extension(library, library_path)
214214
function_name = "Init_#{name}"
215215

216216
init_function = library[function_name]
217-
begin
218-
Primitive.call_with_c_mutex_and_frame(VOID_TO_VOID_WRAPPER, [init_function], nil, nil)
219-
ensure
220-
resolve_registered_addresses
221-
end
217+
218+
Primitive.call_with_c_mutex_and_frame(-> {
219+
begin
220+
Primitive.interop_execute(VOID_TO_VOID_WRAPPER, [init_function])
221+
ensure
222+
# Resolve while inside the ExtensionCallStackEntry to ensure the preservedObjects are still all alive
223+
resolve_registered_addresses
224+
end
225+
}, [], nil, nil)
222226
end
223227

224228
def supported?
@@ -2238,15 +2242,19 @@ def rb_gc_register_address(address)
22382242
else
22392243
# Read it immediately if outside Init_ function.
22402244
# This assumes the value is already set when this is called and does not change after that.
2241-
GC_REGISTERED_ADDRESSES[address] = LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address)
2245+
register_address(address)
22422246
end
22432247
end
22442248

22452249
def resolve_registered_addresses
2246-
c_global_variables = Primitive.fiber_c_global_variables
2247-
c_global_variables.each do |address|
2248-
GC_REGISTERED_ADDRESSES[address] = LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address)
2249-
end
2250+
Primitive.fiber_c_global_variables.each { |address| register_address(address) }
2251+
end
2252+
2253+
private def register_address(address)
2254+
# We save the ValueWrapper here and not the actual value/object, this is important for primitives like double and
2255+
# not-fixnum-long, as we need to preserve the handle by preserving the ValueWrapper of that handle.
2256+
# For those cases the primitive cannot itself reference its ValueWrapper, unlike RubyDynamicObject and ImmutableRubyObject.
2257+
GC_REGISTERED_ADDRESSES[address] = Primitive.cext_to_wrapper LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address)
22502258
end
22512259

22522260
def rb_gc_unregister_address(address)

src/main/c/cext/gc.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ void rb_gc_register_mark_object(VALUE obj) {
6969
}
7070

7171
void* rb_tr_read_VALUE_pointer(VALUE *pointer) {
72-
VALUE value = *pointer;
73-
return rb_tr_unwrap(value);
72+
// No rb_tr_unwrap() here as the caller actually wants a ValueWrapper or a handle
73+
return *pointer;
7474
}
7575

7676
int rb_during_gc(void) {

src/main/java/org/truffleruby/cext/CExtNodes.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,11 +1791,10 @@ Object sym2id(RubySymbol symbol,
17911791
public abstract static class WrapValueNode extends PrimitiveArrayArgumentsNode {
17921792

17931793
@Specialization
1794-
Object wrapInt(Object value,
1794+
ValueWrapper wrap(Object value,
17951795
@Cached WrapNode wrapNode) {
17961796
return wrapNode.execute(value);
17971797
}
1798-
17991798
}
18001799

18011800
@Primitive(name = "cext_unwrap")
@@ -1820,6 +1819,21 @@ private String exceptionMessage(Object value) {
18201819
}
18211820
}
18221821

1822+
@Primitive(name = "cext_to_wrapper")
1823+
public abstract static class CExtToWrapperNode extends PrimitiveArrayArgumentsNode {
1824+
@Specialization
1825+
ValueWrapper toWrapper(Object value,
1826+
@Cached UnwrapNode.ToWrapperNode toWrapperNode) {
1827+
ValueWrapper wrapper = toWrapperNode.execute(this, value);
1828+
if (wrapper == null) {
1829+
CompilerDirectives.transferToInterpreterAndInvalidate();
1830+
throw CompilerDirectives.shouldNotReachHere("ValueWrapper not found for " + value);
1831+
}
1832+
return wrapper;
1833+
}
1834+
}
1835+
1836+
18231837
@CoreMethod(names = "create_mark_list", onSingleton = true, required = 1)
18241838
public abstract static class NewMarkerList extends CoreMethodArrayArgumentsNode {
18251839

@@ -1837,7 +1851,7 @@ Object createNewMarkList(RubyDynamicObject object,
18371851
public abstract static class AddToMarkList extends CoreMethodArrayArgumentsNode {
18381852

18391853
@Specialization
1840-
Object addToMarkList(Object markedObject,
1854+
Object rbGCMark(Object markedObject,
18411855
@Cached InlinedBranchProfile noExceptionProfile,
18421856
@Cached UnwrapNode.ToWrapperNode toWrapperNode) {
18431857
ValueWrapper wrappedValue = toWrapperNode.execute(this, markedObject);

0 commit comments

Comments
 (0)