From c76348e5ffe5e519c955af39e9fd798008ed84a2 Mon Sep 17 00:00:00 2001 From: dkimitsa Date: Tue, 14 Oct 2025 11:25:28 +0300 Subject: [PATCH] * fixed: ObjC -- concurrency issues in heavy-duty environment 1. There was a classic deadlock with two locks: * thread A: CUSTOM_OBJECTS, objcBridgeLock * thread B: objcBridgeLock, CUSTOM_OBJECTS 2. dealloc and CUSTOM_OBJECTS.remove was not atomic, as result between these two actions, after dealloc another thread might allocate new peer with same address and inserts into CUSTOM_OBJECTS. next CUSTOM_OBJECTS will drop not related Java object that will cause it to be GCed (if not retained anywhere) 3. peers map keeps week reference to objects. as result due GC it got wiped. this lead to bug in scenario native-to-jave call that `getPeerObject` is not able to find Java object and it is being allocated. This is an issue for custom classes where Java object keeps state. In this case it is being lost as blank object is created. --- .../main/java/org/robovm/objc/ObjCObject.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/compiler/objc/src/main/java/org/robovm/objc/ObjCObject.java b/compiler/objc/src/main/java/org/robovm/objc/ObjCObject.java index 215263129..338321fb2 100755 --- a/compiler/objc/src/main/java/org/robovm/objc/ObjCObject.java +++ b/compiler/objc/src/main/java/org/robovm/objc/ObjCObject.java @@ -182,6 +182,12 @@ protected static T getPeerObject(long handle) { synchronized (objcBridgeLock) { ObjCObjectRef ref = peers.get(handle); T o = ref != null ? (T) ref.get() : null; + if (o == null) { + // week reference might be empty due GC but there might be Custom object for the peer in + // strong ObjectOwnershipHelper registry + o = ObjectOwnershipHelper.getPeerObject(handle); + if (o != null) peers.put(handle, new ObjCObjectRef(o)); + } return o; } } @@ -416,7 +422,7 @@ public ObjCObjectRef(ObjCObject referent) { } static class ObjectOwnershipHelper { - private static final LongMap CUSTOM_OBJECTS = new LongMap<>(); + private static final LongMap CUSTOM_OBJECTS = new LongMap<>(); private static final long retainCount = Selector.register("retainCount").getHandle(); private static final long retain = Selector.register("retain").getHandle(); @@ -495,7 +501,7 @@ private static void registerCallbackMethod(long cls, long selector, long newSele * @param self pointer from native part */ public static void retainObject(long self) { - synchronized (CUSTOM_OBJECTS) { + synchronized (objcBridgeLock) { ObjCObject obj = ObjCObject.getPeerObject(self); CUSTOM_OBJECTS.put(self, obj); } @@ -522,7 +528,7 @@ private static void release(@Pointer long self, @Pointer long sel) { // as there is direct retain in afterMarshaled for custom objects int count = ObjCRuntime.int_objc_msgSend(self, retainCount); if (count <= 2) { - synchronized (CUSTOM_OBJECTS) { + synchronized (objcBridgeLock) { // at this moment there is no reference kept for java object // and it is subject for GC if not being referenced anywhere // once GC comes it will cause release() to be called in dispose @@ -545,20 +551,34 @@ private static void dealloc(@Pointer long self, @Pointer long sel) { // mechanism. So let it to deallow with extra retains long cls = ObjCRuntime.object_getClass(self); Super sup = new Super(self, getNativeSuper(cls)); - ObjCRuntime.void_objc_msgSendSuper(sup.getHandle(), sel); - - // and after this remove this peer (as it could be added again due unexpected retain calls) - synchronized (CUSTOM_OBJECTS) { + synchronized (objcBridgeLock) { + // calling [super dealloc] while locked as it + CUSTOM_OBJECTS.remove(self) should be atomic due + // following + // 1. custom dealloc code might trigger self to be retained and added back to CUSTOM_OBJECTS + // CUSTOM_OBJECTS has to be done after dealloc + // 2. after dealloc and between CUSTOM_OBJECTS.remove another thread might allocate same + // memory and already put into CUSTOM_OBJECTS. As result CUSTOM_OBJECTS.remove(self) + // will drop completely different object. synchronized (getPeerObject) is supposed to + // protect against it (but it might slow things down) + ObjCRuntime.void_objc_msgSendSuper(sup.getHandle(), sel); + // and after this remove this peer (as it could be added again due unexpected retain calls) CUSTOM_OBJECTS.remove(self); } } public static boolean isObjectRetained(ObjCObject object) { - synchronized (CUSTOM_OBJECTS) { + synchronized (objcBridgeLock) { return CUSTOM_OBJECTS.containsKey(object.getHandle()); } } + /** + * shall be called with objcBridgeLock locked + */ + static T getPeerObject(long handle) { + return (T)CUSTOM_OBJECTS.get(handle); + } + private static long getNativeSuper(final long cls) { /* * We cannot just assume that cls is a custom class that has an