From b3aabbf558b491adb16d0d4b3a28b3c91781bb79 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 26 Jan 2023 16:05:44 +0100 Subject: [PATCH] Support writing to RData.dfree for native extensions * Fixes https://github.com/oracle/truffleruby/issues/2830 * Fixes https://github.com/oracle/truffleruby/issues/2732 * Fixes https://github.com/oracle/truffleruby/issues/2165 --- CHANGELOG.md | 1 + lib/truffle/truffle/cext.rb | 4 ++-- lib/truffle/truffle/cext_structs.rb | 13 +++++++++--- .../java/org/truffleruby/cext/CExtNodes.java | 11 +++++++++- .../java/org/truffleruby/cext/DataHolder.java | 13 +++++++++++- .../core/DataObjectFinalizationService.java | 20 ++++++++++--------- .../core/DataObjectFinalizerReference.java | 3 --- .../core/objectspace/ObjectSpaceNodes.java | 6 ++---- .../objects/shared/WriteBarrierNode.java | 1 - 9 files changed, 48 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98a0feea623e..ca1bae4f017f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,7 @@ Compatibility: * Implement `Array#intersect?` (#2831, @nirvdrum). * Record the source location in the constant for the `module`/`class` keywords (#2833, @eregon). * Fix `File.open` and support `flags` option (#2820, @andrykonchin). +* Support writing to `RData.dfree` for native extensions (#2830, #2732, #2165, @eregon). Performance: diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index 6908cdd9a405..e1601f6816b7 100644 --- a/lib/truffle/truffle/cext.rb +++ b/lib/truffle/truffle/cext.rb @@ -1481,7 +1481,7 @@ def rb_data_object_wrap(ruby_class, data, mark, free) data_holder = Primitive.data_holder_create(data, mark, free) Primitive.object_hidden_var_set object, DATA_HOLDER, data_holder - Primitive.object_space_define_data_finalizer object, free, data_holder unless Truffle::Interop.null?(free) + Primitive.object_space_define_data_finalizer object, data_holder unless Truffle::Interop.null?(free) define_marker object, mark @@ -1496,7 +1496,7 @@ def rb_data_typed_object_wrap(ruby_class, data, data_type, mark, free, size) Primitive.object_hidden_var_set object, DATA_HOLDER, data_holder Primitive.object_hidden_var_set object, DATA_MEMSIZER, data_sizer(size, data_holder) unless Truffle::Interop.null?(size) - Primitive.object_space_define_data_finalizer object, free, data_holder unless Truffle::Interop.null?(free) + Primitive.object_space_define_data_finalizer object, data_holder unless Truffle::Interop.null?(free) define_marker object, mark diff --git a/lib/truffle/truffle/cext_structs.rb b/lib/truffle/truffle/cext_structs.rb index 5a3b0edb2e10..22b7f9d48daa 100644 --- a/lib/truffle/truffle/cext_structs.rb +++ b/lib/truffle/truffle/cext_structs.rb @@ -87,8 +87,15 @@ def polyglot_read_member(name) end def polyglot_write_member(name, value) - raise Truffle::Interop::UnknownIdentifierException unless name == 'data' - Primitive.data_holder_set_data(@data_holder, value) + case name + when 'data' + Primitive.data_holder_set_data(@data_holder, value) + when 'dfree' + Primitive.data_holder_set_free(@data_holder, value) + else + raise Truffle::Interop::UnknownIdentifierException + end + end def polyglot_remove_member(name) @@ -104,7 +111,7 @@ def polyglot_member_readable?(name) end def polyglot_member_modifiable?(name) - name == 'data' + name == 'data' or name == 'dfree' end def polyglot_member_removable?(name) diff --git a/src/main/java/org/truffleruby/cext/CExtNodes.java b/src/main/java/org/truffleruby/cext/CExtNodes.java index 834180126027..0b9d93558ff4 100644 --- a/src/main/java/org/truffleruby/cext/CExtNodes.java +++ b/src/main/java/org/truffleruby/cext/CExtNodes.java @@ -2116,11 +2116,20 @@ protected Object getMarker(Nil data) { @Primitive(name = "data_holder_get_free") public abstract static class DataHolderGetFree extends PrimitiveArrayArgumentsNode { @Specialization - protected Object getMarker(DataHolder data) { + protected Object getFree(DataHolder data) { return data.getFree(); } } + @Primitive(name = "data_holder_set_free") + public abstract static class DataHolderSetFree extends PrimitiveArrayArgumentsNode { + @Specialization + protected Object setFree(DataHolder data, Object dfree) { + data.setFree(dfree); + return dfree; + } + } + @Primitive(name = "data_holder_is_holder?") public abstract static class DataHolderIsHolder extends PrimitiveArrayArgumentsNode { diff --git a/src/main/java/org/truffleruby/cext/DataHolder.java b/src/main/java/org/truffleruby/cext/DataHolder.java index be286c1a2a29..1815ee2d4c18 100644 --- a/src/main/java/org/truffleruby/cext/DataHolder.java +++ b/src/main/java/org/truffleruby/cext/DataHolder.java @@ -18,6 +18,7 @@ import org.truffleruby.RubyLanguage; import org.truffleruby.interop.TranslateInteropExceptionNode; +import org.truffleruby.language.RubyGuards; /** This object represents a struct pointer in C held by a Ruby object. */ @ExportLibrary(InteropLibrary.class) @@ -25,9 +26,13 @@ public final class DataHolder implements TruffleObject { private Object pointer; private final Object marker; - private final Object free; + private Object free; public DataHolder(Object pointer, Object marker, Object free) { + // All foreign objects, so we don't need to share them for SharedObjects + assert RubyGuards.isForeignObject(pointer); + assert RubyGuards.isForeignObject(marker); + assert RubyGuards.isForeignObject(free); this.pointer = pointer; this.marker = marker; this.free = free; @@ -38,6 +43,7 @@ public Object getPointer() { } public void setPointer(Object pointer) { + assert RubyGuards.isForeignObject(pointer); this.pointer = pointer; } @@ -49,6 +55,11 @@ public Object getFree() { return free; } + public void setFree(Object free) { + assert RubyGuards.isForeignObject(free); + this.free = free; + } + @ExportMessage protected boolean hasLanguage() { return true; diff --git a/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java b/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java index 11e102119496..ccd35537b2f6 100644 --- a/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java +++ b/src/main/java/org/truffleruby/core/DataObjectFinalizationService.java @@ -38,7 +38,8 @@ public class DataObjectFinalizationService extends ReferenceProcessingService { - public final Object callable; public final DataHolder dataHolder; DataObjectFinalizerReference( Object object, ReferenceQueue queue, DataObjectFinalizationService service, - Object callable, DataHolder dataHolder) { super(object, queue, service); - this.callable = callable; this.dataHolder = dataHolder; } } diff --git a/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java b/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java index d5fd65b4ac84..671f193ddcc7 100644 --- a/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java +++ b/src/main/java/org/truffleruby/core/objectspace/ObjectSpaceNodes.java @@ -260,8 +260,7 @@ private void defineFinalizer(RubyDynamicObject object, Object finalizer) { public abstract static class DefineDataObjectFinalizerNode extends PrimitiveArrayArgumentsNode { @Specialization - protected Object defineFinalizer( - VirtualFrame frame, RubyDynamicObject object, Object finalizer, DataHolder dataHolder, + protected Object defineFinalizer(RubyDynamicObject object, DataHolder dataHolder, @Cached WriteBarrierNode writeBarrierNode, @CachedLibrary(limit = "1") DynamicObjectLibrary objectLibrary) { if (!getContext().getReferenceProcessor().processOnMainThread()) { @@ -269,13 +268,12 @@ protected Object defineFinalizer( if (!getContext().getSharedObjects().isSharing()) { startSharing(); } - writeBarrierNode.executeWriteBarrier(finalizer); writeBarrierNode.executeWriteBarrier(dataHolder); } DataObjectFinalizerReference newRef = getContext() .getDataObjectFinalizationService() - .addFinalizer(getContext(), object, finalizer, dataHolder); + .addFinalizer(getContext(), object, dataHolder); objectLibrary.put(object, Layouts.DATA_OBJECT_FINALIZER_REF_IDENTIFIER, newRef); diff --git a/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java b/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java index 511b556d006f..7c0059162387 100644 --- a/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java +++ b/src/main/java/org/truffleruby/language/objects/shared/WriteBarrierNode.java @@ -96,7 +96,6 @@ protected void writeBarrierFinalizer(FinalizerReference ref) { @Specialization @TruffleBoundary protected void writeBarrierDataFinalizer(DataObjectFinalizerReference ref) { - SharedObjects.writeBarrier(getLanguage(), ref.callable); SharedObjects.writeBarrier(getLanguage(), ref.dataHolder); }