Skip to content

Commit a57a528

Browse files
committed
Fix: native object reference may already be invalid.
1 parent 08392ce commit a57a528

File tree

2 files changed

+23
-20
lines changed

2 files changed

+23
-20
lines changed

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/CApiContext.java

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -491,34 +491,37 @@ public PythonAbstractNativeObject getPythonNativeObject(TruffleObject nativePtr,
491491
int id = CApiContext.idFromRefCnt(getObRefCntNode.execute(nativePtr));
492492

493493
NativeObjectReference ref;
494-
PythonAbstractNativeObject nativeObject;
495494

496495
// If there is no mapping, we need to create a new one.
497496
if (newRefProfile.profile(id == 0)) {
498-
nativeObject = createPythonAbstractNativeObject(nativePtr, addRefCntNode, steal);
497+
return createPythonAbstractNativeObject(nativePtr, addRefCntNode, steal);
499498
} else if (validRefProfile.profile(id > 0)) {
499+
PythonAbstractNativeObject nativeObject;
500500
ref = lookupNativeObjectReference(id);
501-
nativeObject = ref.get();
502-
if (resurrectProfile.profile(nativeObject == null)) {
503-
// Bad luck: the mapping is still there and wasn't cleaned up but we need a new
504-
// mapping. Therefore, we need to cancel the cleaner action and set a new native
505-
// object reference.
506-
ref.markAsResurrected();
507-
nativeObject = new PythonAbstractNativeObject(nativePtr);
508-
assert id == ref.id;
509-
510-
ref = new NativeObjectReference(nativeObject, nativeObjectsQueue, ref.managedRefCount, id);
511-
NativeObjectReference old = nativeObjectWrapperList.resurrect(id, ref);
512-
assert isReferenceToSameNativeObject(old, ref) : "resurrected native object reference does not point to same native object";
513-
}
514-
if (steal) {
515-
ref.managedRefCount++;
501+
if (ref != null) {
502+
nativeObject = ref.get();
503+
if (resurrectProfile.profile(nativeObject == null)) {
504+
// Bad luck: the mapping is still there and wasn't cleaned up but we need a new
505+
// mapping. Therefore, we need to cancel the cleaner action and set a new native
506+
// object reference.
507+
ref.markAsResurrected();
508+
nativeObject = new PythonAbstractNativeObject(nativePtr);
509+
assert id == ref.id;
510+
511+
ref = new NativeObjectReference(nativeObject, nativeObjectsQueue, ref.managedRefCount, id);
512+
NativeObjectReference old = nativeObjectWrapperList.resurrect(id, ref);
513+
assert isReferenceToSameNativeObject(old, ref) : "resurrected native object reference does not point to same native object";
514+
}
515+
if (steal) {
516+
ref.managedRefCount++;
517+
}
518+
return nativeObject;
516519
}
520+
return createPythonAbstractNativeObject(nativePtr, addRefCntNode, steal);
517521
} else {
518522
LOGGER.warning(() -> String.format("cannot associate a native object reference to %s because reference count is corrupted", CApiContext.asHex(nativePtr)));
519-
nativeObject = new PythonAbstractNativeObject(nativePtr);
520523
}
521-
return nativeObject;
524+
return new PythonAbstractNativeObject(nativePtr);
522525
}
523526

524527
PythonAbstractNativeObject createPythonAbstractNativeObject(TruffleObject nativePtr, AddRefCntNode addRefCntNode, boolean steal) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/capi/NativeReferenceCache.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ private static Object lookupNativeObjectReference(Object pointerObject, int idx,
192192
if (wrapperExistsProfile.profile(idx > 0)) {
193193
NativeObjectReference ref = cApiContext.lookupNativeObjectReference(idx);
194194

195-
PythonAbstractObject object = ref.get();
195+
PythonAbstractObject object = ref != null ? ref.get() : null;
196196
if (object != null) {
197197
// If this is stealing the reference, we need to fixup the managed reference
198198
// count.

0 commit comments

Comments
 (0)