Skip to content

Commit

Permalink
Fix rb_global_variable() for Float and bignum values during the Init_…
Browse files Browse the repository at this point in the history
… function

* We need to store the ValueWrapper and not the value/object itself in GC_REGISTERED_ADDRESSES,
  see #3478 (comment)
* Fixes #3478
  • Loading branch information
eregon committed Mar 25, 2024
1 parent e7eb8ea commit f40067b
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Bug fixes:
* Add missing thread-safe objects write barriers for `TruffleRuby::ConcurrentMap` (#3179, @eregon).
* Fix repeated calling of methods `Dir#{each,each_child,children}` (#3464, @andrykonchin).
* Fix `IO#{wait,wait_readable,wait_writable}` methods and switch the current thread into a sleep state (@andrykonchin).
* Fix `rb_global_variable()` for `Float` and bignum values during the `Init_` function (#3478, @eregon).

Compatibility:
* Move `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core library (@andrykonchin).
Expand Down
2 changes: 1 addition & 1 deletion lib/cext/ABI_check.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
14
15
28 changes: 18 additions & 10 deletions lib/truffle/truffle/cext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,15 @@ def init_extension(library, library_path)
function_name = "Init_#{name}"

init_function = library[function_name]
begin
Primitive.call_with_c_mutex_and_frame(VOID_TO_VOID_WRAPPER, [init_function], nil, nil)
ensure
resolve_registered_addresses
end

Primitive.call_with_c_mutex_and_frame(-> {
begin
Primitive.interop_execute(VOID_TO_VOID_WRAPPER, [init_function])
ensure
# Resolve while inside the ExtensionCallStackEntry to ensure the preservedObjects are still all alive
resolve_registered_addresses
end
}, [], nil, nil)
end

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

def resolve_registered_addresses
c_global_variables = Primitive.fiber_c_global_variables
c_global_variables.each do |address|
GC_REGISTERED_ADDRESSES[address] = LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address)
end
Primitive.fiber_c_global_variables.each { |address| register_address(address) }
end

private def register_address(address)
# We save the ValueWrapper here and not the actual value/object, this is important for primitives like double and
# not-fixnum-long, as we need to preserve the handle by preserving the ValueWrapper of that handle.
# For those cases the primitive cannot itself reference its ValueWrapper, unlike RubyDynamicObject and ImmutableRubyObject.
GC_REGISTERED_ADDRESSES[address] = Primitive.cext_to_wrapper LIBTRUFFLERUBY.rb_tr_read_VALUE_pointer(address)
end

def rb_gc_unregister_address(address)
Expand Down
4 changes: 2 additions & 2 deletions src/main/c/cext/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ void rb_gc_register_mark_object(VALUE obj) {
}

void* rb_tr_read_VALUE_pointer(VALUE *pointer) {
VALUE value = *pointer;
return rb_tr_unwrap(value);
// No rb_tr_unwrap() here as the caller actually wants a ValueWrapper or a handle
return *pointer;
}

int rb_during_gc(void) {
Expand Down
20 changes: 17 additions & 3 deletions src/main/java/org/truffleruby/cext/CExtNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -1791,11 +1791,10 @@ Object sym2id(RubySymbol symbol,
public abstract static class WrapValueNode extends PrimitiveArrayArgumentsNode {

@Specialization
Object wrapInt(Object value,
ValueWrapper wrap(Object value,
@Cached WrapNode wrapNode) {
return wrapNode.execute(value);
}

}

@Primitive(name = "cext_unwrap")
Expand All @@ -1820,6 +1819,21 @@ private String exceptionMessage(Object value) {
}
}

@Primitive(name = "cext_to_wrapper")
public abstract static class CExtToWrapperNode extends PrimitiveArrayArgumentsNode {
@Specialization
ValueWrapper toWrapper(Object value,
@Cached UnwrapNode.ToWrapperNode toWrapperNode) {
ValueWrapper wrapper = toWrapperNode.execute(this, value);
if (wrapper == null) {
CompilerDirectives.transferToInterpreterAndInvalidate();
throw CompilerDirectives.shouldNotReachHere("ValueWrapper not found for " + value);
}
return wrapper;
}
}


@CoreMethod(names = "create_mark_list", onSingleton = true, required = 1)
public abstract static class NewMarkerList extends CoreMethodArrayArgumentsNode {

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

@Specialization
Object addToMarkList(Object markedObject,
Object rbGCMark(Object markedObject,
@Cached InlinedBranchProfile noExceptionProfile,
@Cached UnwrapNode.ToWrapperNode toWrapperNode) {
ValueWrapper wrappedValue = toWrapperNode.execute(this, markedObject);
Expand Down

0 comments on commit f40067b

Please sign in to comment.