Skip to content

Commit

Permalink
Migrate StablePtr to KRefSharedHolder (JetBrains#3966)
Browse files Browse the repository at this point in the history
This PR also fixes tests from JetBrains#3969
  • Loading branch information
projedi authored Mar 12, 2020
1 parent d91266d commit 02eb631
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 12 deletions.
4 changes: 4 additions & 0 deletions backend.native/tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2742,6 +2742,10 @@ standaloneTest("memory_only_gc") {
source = "runtime/memory/only_gc.kt"
}

task memory_stable_ref_cross_thread_check(type: KonanLocalTest) {
source = "runtime/memory/stable_ref_cross_thread_check.kt"
}

standaloneTest("cycle_collector") {
disabled = project.globalTestArgs.contains('-opt') || (project.testTarget == 'wasm32') // Needs debug build.
flags = ['-g']
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
* that can be found in the LICENSE file.
*/

package runtime.memory.stable_ref_cross_thread_check

import kotlin.test.*

import kotlin.native.concurrent.*
import kotlinx.cinterop.*

@Test
fun runTest1() {
val worker = Worker.start()

val future = worker.execute(TransferMode.SAFE, { }) {
StableRef.create(Any())
}
val ref = future.result
assertFailsWith<IncorrectDereferenceException> {
val value = ref.get()
println(value.toString())
}

worker.requestTermination().result
}

@Test
fun runTest2() {
val worker = Worker.start()

val mainThreadRef = StableRef.create(Any())
// Simulate this going through interop as raw C pointer.
val pointerValue: Long = mainThreadRef.asCPointer().toLong()
val future = worker.execute(TransferMode.SAFE, { pointerValue }) {
val pointer: COpaquePointer = it.toCPointer()!!
assertFailsWith<IncorrectDereferenceException> {
// Even attempting to convert a pointer to StableRef should fail.
val otherThreadRef: StableRef<Any> = pointer.asStableRef()
println(otherThreadRef.toString())
}
Unit
}
future.result

worker.requestTermination().result
}
13 changes: 10 additions & 3 deletions runtime/src/main/cpp/Interop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,28 @@
#include <stdio.h>
#include <stdint.h>

#include "Alloc.h"
#include "Memory.h"
#include "MemorySharedRefs.hpp"
#include "Types.h"

extern "C" {

KNativePtr Kotlin_Interop_createStablePointer(KRef any) {
return CreateStablePointer(any);
KRefSharedHolder* holder = konanConstructInstance<KRefSharedHolder>();
holder->init(any);
return holder;
}

void Kotlin_Interop_disposeStablePointer(KNativePtr pointer) {
DisposeStablePointer(pointer);
KRefSharedHolder* holder = reinterpret_cast<KRefSharedHolder*>(pointer);
holder->dispose();
konanDestructInstance(holder);
}

OBJ_GETTER(Kotlin_Interop_derefStablePointer, KNativePtr pointer) {
RETURN_RESULT_OF(DerefStablePointer, pointer);
KRefSharedHolder* holder = reinterpret_cast<KRefSharedHolder*>(pointer);
RETURN_OBJ(holder->ref());
}

}
12 changes: 8 additions & 4 deletions runtime/src/main/cpp/Memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2279,10 +2279,7 @@ void disposeStablePointer(KNativePtr pointer) {

OBJ_GETTER(derefStablePointer, KNativePtr pointer) {
KRef ref = reinterpret_cast<KRef>(pointer);
#if USE_GC
if (IsStrictMemoryModel && ref != nullptr)
rememberNewContainer(ref->container());
#endif // USE_GC
AdoptReferenceFromSharedVariable(ref);
RETURN_OBJ(ref);
}

Expand Down Expand Up @@ -2758,6 +2755,13 @@ bool IsForeignRefAccessible(ObjHeader* object, ForeignRefContext context) {
return isForeignRefAccessible(object, context);
}

void AdoptReferenceFromSharedVariable(ObjHeader* object) {
#if USE_GC
if (IsStrictMemoryModel && object != nullptr && isShareable(object->container()))
rememberNewContainer(object->container());
#endif // USE_GC
}

// Public memory interface.
MemoryState* InitMemory() {
return initMemory();
Expand Down
4 changes: 4 additions & 0 deletions runtime/src/main/cpp/MemoryPrivate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ void DeinitForeignRef(ObjHeader* object, ForeignRefContext context);

bool IsForeignRefAccessible(ObjHeader* object, ForeignRefContext context);

// Should be used when reference is read from a possibly shared variable,
// and there's nothing else keeping the object alive.
void AdoptReferenceFromSharedVariable(ObjHeader* object);

} // extern "C"

#endif // RUNTIME_MEMORYPRIVATE_HPP
10 changes: 9 additions & 1 deletion runtime/src/main/cpp/MemorySharedRefs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ void KRefSharedHolder::init(ObjHeader* obj) {

ObjHeader* KRefSharedHolder::ref() const {
ensureRefAccessible();
AdoptReferenceFromSharedVariable(obj_);
return obj_;
}

Expand Down Expand Up @@ -76,7 +77,8 @@ void BackRefFromAssociatedObject::addRef() {

bool BackRefFromAssociatedObject::tryAddRef() {
// Suboptimal but simple:
ObjHeader* obj = this->ref();
this->ensureRefAccessible();
ObjHeader* obj = this->obj_;

if (!TryAddHeapRef(obj)) return false;
this->addRef();
Expand All @@ -97,6 +99,12 @@ void BackRefFromAssociatedObject::releaseRef() {
}
}

ObjHeader* BackRefFromAssociatedObject::ref() const {
ensureRefAccessible();
AdoptReferenceFromSharedVariable(obj_);
return obj_;
}

void BackRefFromAssociatedObject::ensureRefAccessible() const {
ensureForeignRefAccessible(obj_, context_);
}
Expand Down
5 changes: 1 addition & 4 deletions runtime/src/main/cpp/MemorySharedRefs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ class BackRefFromAssociatedObject {

void releaseRef();

ObjHeader* ref() const {
ensureRefAccessible();
return obj_;
}
ObjHeader* ref() const;

inline bool permanent() const {
return obj_->permanent(); // Safe to query from any thread.
Expand Down

0 comments on commit 02eb631

Please sign in to comment.