Don't allow GC in BackRefFromAssociatedObject::tryAddRef

This function is invoked while a non-recursive os_lock is taken,
and GC can provoke Obj-C object deallocation taking the same lock
recursively.
This commit is contained in:
SvyatoslavScherbina
2020-10-06 19:28:00 +03:00
committed by GitHub
parent 93df6c45ac
commit 34cd218e4a
8 changed files with 63 additions and 5 deletions
@@ -0,0 +1,6 @@
#import <Foundation/NSObject.h>
@interface WeakRefHolder : NSObject
@property (weak) id obj;
-(void)loadManyTimes;
@end;
@@ -0,0 +1,25 @@
import kotlinx.cinterop.*
import kotlin.test.*
import objcTests.*
@Test
fun testTryRetainGC() {
kotlin.native.internal.GC.collect()
val weakRefHolder = WeakRefHolder()
createGarbageNSObjects(weakRefHolder)
weakRefHolder.obj = object : NSObject() {}
// Loading weak ref takes a lock. If K/N runtime runs GC while the lock is taken,
// then it releases garbage objects and thus Obj-C runtime might take a recursive lock
// and abort in _os_unfair_lock_recursive_abort.
weakRefHolder.loadManyTimes()
}
private fun createGarbageNSObjects(weakRefHolder: WeakRefHolder) {
autoreleasepool {
repeat(100) {
// Assigning the object to a weak reference so Obj-C would take a lock when deallocating it:
weakRefHolder.obj = NSObject()
}
weakRefHolder.obj = null
}
}
@@ -0,0 +1,11 @@
#import "tryRetainGC.h"
#import <Foundation/NSArray.h>
@implementation WeakRefHolder
-(void)loadManyTimes {
NSMutableArray* array = [NSMutableArray new];
for (int i = 0; i < 10000; ++i) {
[array addObject:self.obj];
}
}
@end;
+11 -4
View File
@@ -1616,23 +1616,23 @@ inline bool tryAddHeapRef(const ObjHeader* header) {
return (container != nullptr) ? tryAddHeapRef(container) : true;
}
template <bool Strict>
template <bool Strict, bool CanCollect>
inline void releaseHeapRef(ContainerHeader* container) {
MEMORY_LOG("ReleaseHeapRef %p: rc=%d\n", container, container->refCount())
UPDATE_RELEASEREF_STAT(memoryState, container, needAtomicAccess(container), canBeCyclic(container), 0)
if (container->tag() != CONTAINER_TAG_STACK) {
if (Strict)
enqueueDecrementRC</* CanCollect = */ true>(container);
enqueueDecrementRC</* CanCollect = */ CanCollect>(container);
else
decrementRC(container);
}
}
template <bool Strict>
template <bool Strict, bool CanCollect = true>
inline void releaseHeapRef(const ObjHeader* header) {
auto* container = header->container();
if (container != nullptr)
releaseHeapRef<Strict>(const_cast<ContainerHeader*>(container));
releaseHeapRef<Strict, CanCollect>(const_cast<ContainerHeader*>(container));
}
// We use first slot as place to store frame-local arena container.
@@ -3135,6 +3135,13 @@ void ReleaseHeapRefRelaxed(const ObjHeader* object) {
releaseHeapRef<false>(const_cast<ObjHeader*>(object));
}
void ReleaseHeapRefNoCollectStrict(const ObjHeader* object) {
releaseHeapRef<true, /* CanCollect = */ false>(const_cast<ObjHeader*>(object));
}
void ReleaseHeapRefNoCollectRelaxed(const ObjHeader* object) {
releaseHeapRef<false, /* CanCollect = */ false>(const_cast<ObjHeader*>(object));
}
ForeignRefContext InitLocalForeignRef(ObjHeader* object) {
return initLocalForeignRef(object);
}
+1
View File
@@ -24,6 +24,7 @@ extern "C" {
bool TryAddHeapRef(const ObjHeader* object);
MODEL_VARIANTS(void, ReleaseHeapRef, const ObjHeader* object);
MODEL_VARIANTS(void, ReleaseHeapRefNoCollect, const ObjHeader* object);
void Kotlin_ObjCExport_releaseAssociatedObject(void* associatedObject);
+1 -1
View File
@@ -153,7 +153,7 @@ bool BackRefFromAssociatedObject::tryAddRef() {
RuntimeAssert(isForeignRefAccessible(obj_, context_), "Cannot be inaccessible because of the check above");
// TODO: This is a very weird way to ask for "unsafe" addRef.
addRef<ErrorPolicy::kIgnore>();
ReleaseHeapRef(obj); // Balance TryAddHeapRef.
ReleaseHeapRefNoCollect(obj); // Balance TryAddHeapRef.
// TODO: consider optimizing for non-shared objects.
return true;
+4
View File
@@ -33,6 +33,10 @@ void ReleaseHeapRef(const ObjHeader* object) {
ReleaseHeapRefRelaxed(object);
}
void ReleaseHeapRefNoCollect(const ObjHeader* object) {
ReleaseHeapRefNoCollectRelaxed(object);
}
void ZeroStackRef(ObjHeader** location) {
ZeroStackRefRelaxed(location);
}
+4
View File
@@ -33,6 +33,10 @@ void ReleaseHeapRef(const ObjHeader* object) {
ReleaseHeapRefStrict(object);
}
void ReleaseHeapRefNoCollect(const ObjHeader* object) {
ReleaseHeapRefNoCollectStrict(object);
}
void SetStackRef(ObjHeader** location, const ObjHeader* object) {
SetStackRefStrict(location, object);
}