Only call tp_dealloc on objects that were only referenced from managed by now

Author: timfel

graalpython/com.oracle.graal.python.cext/src/capi.c

@@ -409,18 +409,10 @@ GraalPyPrivate_BulkDealloc(intptr_t ptrArray[], int64_t len)
 {
  for (int i = 0; i < len; i++) {
  PyObject *obj = (PyObject *)ptrArray[i];
- if (obj->ob_refcnt == _Py_IMMORTAL_REFCNT) {
- GraalPyPrivate_Log(PY_TRUFFLE_LOG_FINER,
- "%s: Not deallocating immortal object: 0x%zx (%s%s)",
- __func__, obj,
- PyType_Check(obj) ? "class " : "a ",
- PyType_Check(obj) ? ((PyTypeObject*)obj)->tp_name : Py_TYPE(obj)->tp_name);
- } else {
- GraalPyPrivate_Log(PY_TRUFFLE_LOG_FINER,
- "%s: _Py_Dealloc(0x%zx)",
- __func__, obj);
- _Py_Dealloc(obj);
- }
+ GraalPyPrivate_Log(PY_TRUFFLE_LOG_FINER,
+ "%s: _Py_Dealloc(a %s at 0x%zx)",
+ __func__, Py_TYPE(obj)->tp_name, obj);
+ _Py_Dealloc(obj);
  }
  return 0;
 }

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

@@ -653,13 +653,20 @@ public static void deallocNativeReplacements(PythonContext context, HandleContex
  IdReference<?> ref = iterator.next().getValue();
  if (ref instanceof PythonObjectReference reference) {
  if (!reference.isAllocatedFromJava()) {
- // This memory must be free from C
- referencesToBeFreed.add(reference.pointer);
- iterator.remove();
+ // This memory must be freed from C
+ if (subNativeRefCount(reference.pointer, MANAGED_REFCNT) == 0) {
+ // Only the managed references exist, we can dealloc
+ referencesToBeFreed.add(reference.pointer);
+ iterator.remove();
+ } else {
+ // There is a native reference which presumably will decref this one at a
+ // later point and call tp_dealloc then
+ }
  }
  }
  }
  releaseNativeObjects(context, referencesToBeFreed);
+ pollReferenceQueue();
  }
 
  public static void freeNativeReplacementStructs(PythonContext context, HandleContext handleContext) {
@@ -668,9 +675,12 @@ public static void freeNativeReplacementStructs(PythonContext context, HandleCon
  if (ref instanceof PythonObjectReference reference) {
  // We don't expect references to wrappers that would have a native object stub.
  assert reference.handleTableIndex == -1;
- // We expect at this point that there are only references allocated from Java
- assert reference.isAllocatedFromJava();
- freeNativeStruct(reference);
+ // We expect at this point that most if not all references left were allocated
+ // from Java and can be freed here. There may be stragglers that are waiting
+ // for a GC though, so we have to check.
+ if (reference.isAllocatedFromJava()) {
+ freeNativeStruct(reference);
+ }
  }
  });
  handleContext.nativeLookup.clear();