Skip to content

* fixed: ObjC -- concurrency issues in heavy-duty environment #821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dkimitsa wants to merge 1 commit into
base: master
Choose a base branch
from
Open

* fixed: ObjC -- concurrency issues in heavy-duty environment #821

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions compiler/objc/src/main/java/org/robovm/objc/ObjCObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ protected static <T extends ObjCObject> 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;
}
}
Expand Down Expand Up @@ -416,7 +422,7 @@ public ObjCObjectRef(ObjCObject referent) {
}

static class ObjectOwnershipHelper {
private static final LongMap<Object> CUSTOM_OBJECTS = new LongMap<>();
private static final LongMap<ObjCObject> CUSTOM_OBJECTS = new LongMap<>();

private static final long retainCount = Selector.register("retainCount").getHandle();
private static final long retain = Selector.register("retain").getHandle();
Expand Down Expand Up @@ -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);
}
Expand All @@ -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
Expand All @@ -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 extends ObjCObject> 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
Expand Down
Loading