Skip to content

Commit a9e7146

Browse files
aardvark179aardvark179
committed
Changes based on review.
1 parent 2324772 commit a9e7146

File tree

4 files changed

+40
-38
lines changed

4 files changed

+40
-38
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.truffleruby.builtins.Primitive;
3838
import org.truffleruby.builtins.YieldingCoreMethodNode;
3939
import org.truffleruby.cext.CExtNodesFactory.StringToNativeNodeGen;
40-
import org.truffleruby.cext.ValueWrapperObjectType.HandleNotFoundException;
4140
import org.truffleruby.core.CoreLibrary;
4241
import org.truffleruby.core.array.ArrayHelpers;
4342
import org.truffleruby.core.array.ArrayOperations;
@@ -1282,11 +1281,12 @@ public abstract static class UnwrapValueNode extends CoreMethodArrayArgumentsNod
12821281
public Object unwrap(Object value,
12831282
@Cached("create()") BranchProfile exceptionProfile,
12841283
@Cached("createUnwrapNode()") UnwrapNode unwrapNode) {
1285-
try {
1286-
return unwrapNode.execute(value);
1287-
} catch (HandleNotFoundException e) {
1284+
Object object = unwrapNode.execute(value);
1285+
if (object == null) {
12881286
exceptionProfile.enter();
12891287
throw new RaiseException(getContext(), coreExceptions().runtimeError("Native handle not found", this));
1288+
} else {
1289+
return object;
12901290
}
12911291
}
12921292

‎src/main/java/org/truffleruby/cext/UnwrapNode.java

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.oracle.truffle.api.interop.UnsupportedMessageException;
1414
import com.oracle.truffle.api.nodes.Node;
1515
import com.oracle.truffle.api.object.DynamicObject;
16+
import com.oracle.truffle.api.profiles.BranchProfile;
1617

1718
@ImportStatic(Message.class)
1819
public abstract class UnwrapNode extends RubyBaseNode {
@@ -26,25 +27,27 @@ public Object unwrapValue(DynamicObject value) {
2627

2728
@Specialization(guards = "!isWrapper(value)")
2829
public Object unwrapTypeCastObject(TruffleObject value,
29-
@Cached("IS_POINTER.createNode()") Node isPOinterNode,
30-
@Cached("AS_POINTER.createNode()") Node asPOinterNode) {
31-
if (ForeignAccess.sendIsPointer(isPOinterNode, value)) {
30+
@Cached("IS_POINTER.createNode()") Node isPointerNode,
31+
@Cached("AS_POINTER.createNode()") Node asPointerNode,
32+
@Cached("create()") BranchProfile unsupportedProfile,
33+
@Cached("create()") BranchProfile nonPointerProfile) {
34+
if (ForeignAccess.sendIsPointer(isPointerNode, value)) {
35+
long handle = 0;
3236
try {
33-
long handle = ForeignAccess.sendAsPointer(asPOinterNode, value);
34-
return unwrapHandle(handle);
37+
handle = ForeignAccess.sendAsPointer(asPointerNode, value);
3538
} catch (UnsupportedMessageException e) {
39+
unsupportedProfile.enter();
3640
throw new RaiseException(getContext(), coreExceptions().argumentError(e.getMessage(), this, e));
3741
}
42+
return unwrapHandle(handle);
43+
} else {
44+
nonPointerProfile.enter();
45+
throw new RaiseException(getContext(), coreExceptions().argumentError("Not a handle or a pointer", this));
3846
}
39-
return value;
4047
}
4148

42-
public Object unwrapHandle(long handle) {
43-
final Object value = ValueWrapperObjectType.getFromHandleMap(handle);
44-
if (value == null) {
45-
throw new ValueWrapperObjectType.HandleNotFoundException();
46-
}
47-
return value;
49+
private Object unwrapHandle(long handle) {
50+
return ValueWrapperObjectType.getFromHandleMap(handle);
4851
}
4952

5053
public static boolean isWrapper(TruffleObject value) {

‎src/main/java/org/truffleruby/cext/ValueWrapperMessageResolution.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static abstract class ForeignAsPointerNode extends Node {
4848

4949
protected long access(VirtualFrame frame, DynamicObject wrapper) {
5050
long handle = Layouts.VALUE_WRAPPER.getHandle(wrapper);
51-
if (handle == ValueWrapperObjectType.NULL_HANDLE) {
51+
if (handle == ValueWrapperObjectType.UNSET_HANDLE) {
5252
handle = createHandle(wrapper);
5353
}
5454
return handle;
@@ -64,14 +64,18 @@ protected long createHandle(DynamicObject wrapper) {
6464
// Add a finaliser to remove the map entry.
6565
getContext().getFinalizationService().addFinalizer(
6666
wrapper, null, ValueWrapperObjectType.class,
67-
() -> {
68-
ValueWrapperObjectType.removeFromHandleMap(handlePointer.getAddress());
69-
handlePointer.free();
70-
}, null);
67+
finalizer(handlePointer), null);
7168
return handleAddress;
7269
}
7370
}
7471

72+
private static Runnable finalizer(Pointer handle) {
73+
return () -> {
74+
ValueWrapperObjectType.removeFromHandleMap(handle.getAddress());
75+
handle.free();
76+
};
77+
78+
}
7579
public RubyContext getContext() {
7680
if (context == null) {
7781
CompilerDirectives.transferToInterpreterAndInvalidate();

‎src/main/java/org/truffleruby/cext/ValueWrapperObjectType.java

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
public class ValueWrapperObjectType extends ObjectType {
2525

26-
static final int NULL_HANDLE = -1;
26+
static final int UNSET_HANDLE = -1;
2727

2828
private static DynamicObject UNDEF_WRAPPER = null;
2929

@@ -35,16 +35,16 @@ public class ValueWrapperObjectType extends ObjectType {
3535
private static LongHashMap<WeakReference<DynamicObject>> handleMap = new LongHashMap<>(1024);
3636

3737
public static DynamicObject createValueWrapper(Object value) {
38-
return Layouts.VALUE_WRAPPER.createValueWrapper(value, NULL_HANDLE);
38+
return Layouts.VALUE_WRAPPER.createValueWrapper(value, UNSET_HANDLE);
3939
}
4040

4141
public static synchronized DynamicObject createUndefWrapper(NotProvided value) {
42-
return UNDEF_WRAPPER != null ? UNDEF_WRAPPER : (UNDEF_WRAPPER = Layouts.VALUE_WRAPPER.createValueWrapper(value, NULL_HANDLE));
42+
return UNDEF_WRAPPER != null ? UNDEF_WRAPPER : (UNDEF_WRAPPER = Layouts.VALUE_WRAPPER.createValueWrapper(value, UNSET_HANDLE));
4343
}
4444

4545
public static synchronized DynamicObject createBooleanWrapper(boolean value) {
4646
if (value) {
47-
return TRUE_WRAPPER != null ? TRUE_WRAPPER : (TRUE_WRAPPER = Layouts.VALUE_WRAPPER.createValueWrapper(true, NULL_HANDLE));
47+
return TRUE_WRAPPER != null ? TRUE_WRAPPER : (TRUE_WRAPPER = Layouts.VALUE_WRAPPER.createValueWrapper(true, UNSET_HANDLE));
4848
} else {
4949
return FALSE_WRAPPER != null ? FALSE_WRAPPER : (FALSE_WRAPPER = createFalseWrapper());
5050
}
@@ -63,15 +63,15 @@ private static DynamicObject createFalseWrapper() {
6363
public static synchronized DynamicObject createLongWrapper(long value) {
6464
DynamicObject wrapper = longMap.get(value);
6565
if (wrapper == null) {
66-
wrapper = Layouts.VALUE_WRAPPER.createValueWrapper(value, NULL_HANDLE);
66+
wrapper = Layouts.VALUE_WRAPPER.createValueWrapper(value, UNSET_HANDLE);
6767
longMap.put(value, wrapper);
6868
}
6969
return wrapper;
7070
}
7171

7272
@TruffleBoundary
7373
public static synchronized DynamicObject createDoubleWrapper(double value) {
74-
return Layouts.VALUE_WRAPPER.createValueWrapper(value, NULL_HANDLE);
74+
return Layouts.VALUE_WRAPPER.createValueWrapper(value, UNSET_HANDLE);
7575
}
7676

7777
public static synchronized void addToHandleMap(long handle, DynamicObject wrapper) {
@@ -83,7 +83,7 @@ public static synchronized Object getFromHandleMap(long handle) {
8383
WeakReference<DynamicObject> ref = handleMap.get(handle);
8484
DynamicObject object;
8585
if (ref == null) {
86-
throw new Error("Bad handle!");
86+
return null;
8787
}
8888
if ((object = ref.get()) == null) {
8989
return null;
@@ -102,15 +102,16 @@ public static boolean isInstance(TruffleObject receiver) {
102102

103103
@Override
104104
public boolean equals(DynamicObject object, Object other) {
105-
if (!(other instanceof ValueWrapperLayout)) {
105+
if (!(other instanceof DynamicObject) || Layouts.VALUE_WRAPPER.isValueWrapper(other)) {
106106
return false;
107107
}
108108
DynamicObject otherWrapper = (DynamicObject) other;
109+
109110
final long objectHandle = Layouts.VALUE_WRAPPER.getHandle(object);
110111
final long otherHandle = Layouts.VALUE_WRAPPER.getHandle(otherWrapper);
111-
if (objectHandle != NULL_HANDLE &&
112-
objectHandle == otherHandle) {
113-
return true;
112+
if (objectHandle != UNSET_HANDLE &&
113+
otherHandle != UNSET_HANDLE) {
114+
return objectHandle == otherHandle;
114115
}
115116
return Layouts.VALUE_WRAPPER.getObject(object).equals(Layouts.VALUE_WRAPPER.getObject(otherWrapper));
116117
}
@@ -119,10 +120,4 @@ public boolean equals(DynamicObject object, Object other) {
119120
public ForeignAccess getForeignAccessFactory(DynamicObject object) {
120121
return ValueWrapperMessageResolutionForeign.ACCESS;
121122
}
122-
123-
@SuppressWarnings("serial")
124-
public static class HandleNotFoundException extends RuntimeException {
125-
126-
}
127-
128123
}

0 commit comments

Comments
 (0)