Skip to content
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.

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

Check if Native object was closed #584

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ package org.jetbrains.skia.impl
* Peer for a native object.
*/
expect abstract class Managed(ptr: NativePointer, finalizer: NativePointer, managed: Boolean = true) : Native {
override val _ptr: NativePointer

/**
* Free underlying native resource, peer is useless afterwards.
*/
Expand Down
4 changes: 2 additions & 2 deletions skiko/src/commonMain/kotlin/org/jetbrains/skia/impl/Native.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ expect class NativePointer

expect class InteropPointer

expect abstract class Native(ptr: NativePointer) {
internal var _ptr: NativePointer
expect abstract class Native() {
internal abstract val _ptr: NativePointer
internal open fun nativeEquals(other: Native?): Boolean

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import org.jetbrains.skia.ExternalSymbolName
import org.jetbrains.skia.impl.NativePointer
import org.jetbrains.skia.impl.getPtr

class ParagraphCache internal constructor(owner: FontCollection, ptr: NativePointer) : Native(ptr) {
class ParagraphCache internal constructor(owner: FontCollection, ptr: NativePointer) : Native() {
companion object {
init {
staticLoad()
}
}

override val _ptr: NativePointer= ptr

fun abandon() {
try {
_validate()
Expand Down
2 changes: 1 addition & 1 deletion skiko/src/commonTest/kotlin/org/jetbrains/skia/DataTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class DataTest {
it
}

assertEquals(NullPointer, copiedData._ptr)
assertTrue(copiedData.isClosed)
assertEquals(originalDataPtr, originalData._ptr)

assertContentEquals(
Expand Down
34 changes: 23 additions & 11 deletions skiko/src/jsMain/kotlin/org/jetbrains/skia/impl/Managed.js.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,36 @@ private fun unregister(managed: Managed) {
registry.unregister(managed)
}

actual abstract class Managed actual constructor(ptr: NativePointer, finalizer: NativePointer, managed: Boolean) : Native(ptr) {
actual abstract class Managed actual constructor(ptr: NativePointer, finalizer: NativePointer, managed: Boolean) : Native() {
private var cleaner: FinalizationThunk? = null

private var __ptr: NativePointer = ptr.also {
require(it != NullPointer) {
"Can't wrap NullPointer"
}
}

actual override val _ptr: NativePointer get() = __ptr.also {
check(__ptr != NullPointer) {
"Object already closed: ${this::class.simpleName}"
}
}

actual open fun close() {
if (NullPointer == _ptr)
throw RuntimeException("Object already closed: ${this::class.simpleName}, _ptr=$_ptr")
else if (null == cleaner)
throw RuntimeException("Object is not managed, can't close(): ${this::class.simpleName}, _ptr=$_ptr")
else {
unregister(this)
cleaner!!.clean()
cleaner = null
_ptr = 0
check(__ptr != NullPointer) {
"Object already closed: ${this::class.simpleName}"
}
check(null != cleaner) {
"Object is not managed, can't close(): ${this::class.simpleName}, _ptr=$__ptr"
}
unregister(this)
cleaner!!.clean()
cleaner = null
__ptr = 0
}

actual open val isClosed: Boolean
get() = _ptr == NullPointer
get() = __ptr == NullPointer

init {
if (managed) {
Expand Down
9 changes: 2 additions & 7 deletions skiko/src/jsMain/kotlin/org/jetbrains/skia/impl/Native.js.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package org.jetbrains.skia.impl

import org.khronos.webgl.ArrayBufferView

actual abstract class Native actual constructor(ptr: NativePointer) {
actual var _ptr: NativePointer
actual abstract class Native {
actual abstract val _ptr: NativePointer

override fun equals(other: Any?): Boolean {
if (this === other) return true
Expand All @@ -26,11 +26,6 @@ actual abstract class Native actual constructor(ptr: NativePointer) {
actual override fun toString(): String {
return this::class.simpleName + "(_ptr=0x" + _ptr.toString(16) + ")"
}

init {
if (ptr == NullPointer) throw RuntimeException("Can't wrap nullptr")
_ptr = ptr
}
}

internal actual fun reachabilityBarrier(obj: Any?) {}
Expand Down
6 changes: 3 additions & 3 deletions skiko/src/jvmMain/cpp/common/interop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -896,16 +896,16 @@ namespace skija {

namespace impl {
namespace Native {
jfieldID _ptr;
jmethodID _ptr;

void onLoad(JNIEnv* env) {
jclass cls = env->FindClass("org/jetbrains/skia/impl/Native");
_ptr = env->GetFieldID(cls, "_ptr", "J");
_ptr = env->GetMethodID(cls, "getPtr", "()J");
}

void* fromJava(JNIEnv* env, jobject obj, jclass cls) {
if (env->IsInstanceOf(obj, cls)) {
jlong ptr = env->GetLongField(obj, skija::impl::Native::_ptr);
jlong ptr = env->CallLongMethod(obj, skija::impl::Native::_ptr);
return reinterpret_cast<void*>(static_cast<uintptr_t>(ptr));
}
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion skiko/src/jvmMain/cpp/common/interop.hh
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ namespace skija {

namespace impl {
namespace Native {
extern jfieldID _ptr;
extern jmethodID _ptr;
void onLoad(JNIEnv* env);
void onUnload(JNIEnv* env);
void* fromJava(JNIEnv* env, jobject obj, jclass cls);
Expand Down
31 changes: 21 additions & 10 deletions skiko/src/jvmMain/kotlin/org/jetbrains/skia/impl/Managed.jvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,33 @@ import kotlin.concurrent.thread
// Consider using this on all JVM platforms eventually.
actual abstract class Managed actual constructor(
ptr: Long, finalizer: Long, managed: Boolean
) : Native(ptr), AutoCloseable {
) : Native(), AutoCloseable {
private var __ptr: NativePointer = ptr.also {
require(it != NullPointer) {
"Can't wrap NullPointer"
}
}

actual override val _ptr: NativePointer get() = __ptr.also {
check(__ptr != NullPointer) {
"Object already closed: $javaClass"
}
}

actual override fun close() {
if (0L == _ptr)
throw RuntimeException("Object already closed: $javaClass, _ptr=$_ptr")
else if (null == cleanable)
throw RuntimeException("Object is not managed in JVM, can't close(): $javaClass, _ptr=$_ptr")
else {
cleanable!!.clean()
cleanable = null
_ptr = 0
check(__ptr != NullPointer) {
"Object already closed: $javaClass"
}
check(null != cleanable) {
"Object is not managed in JVM, can't close(): $javaClass, _ptr=$__ptr"
}
cleanable!!.clean()
cleanable = null
__ptr = 0
}

actual open val isClosed: Boolean
get() = _ptr == 0L
get() = __ptr == 0L

class CleanerThunk(var className: String, var ptr: Long, var finalizerPtr: Long) : Runnable {
override fun run() {
Expand Down
11 changes: 4 additions & 7 deletions skiko/src/jvmMain/kotlin/org/jetbrains/skia/impl/Native.jvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package org.jetbrains.skia.impl

import java.lang.ref.Reference

actual abstract class Native actual constructor(ptr: NativePointer) {
internal actual var _ptr: NativePointer
actual abstract class Native {
internal actual abstract val _ptr: NativePointer

fun getPtr() = _ptr

actual companion object {
actual val NullPointer: NativePointer
Expand Down Expand Up @@ -35,11 +37,6 @@ actual abstract class Native actual constructor(ptr: NativePointer) {
internal actual open fun nativeEquals(other: Native?): Boolean {
return false
}

init {
if (ptr == NullPointer) throw RuntimeException("Can't wrap nullptr")
_ptr = ptr
}
}

internal actual fun reachabilityBarrier(obj: Any?) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private class FinalizationThunk(private val finalizer: NativePointer, val classN
}

actual abstract class Managed actual constructor(
ptr: NativePointer, finalizer: NativePointer, managed: Boolean) : Native(ptr) {
ptr: NativePointer, finalizer: NativePointer, managed: Boolean) : Native() {

private val thunk: FinalizationThunk? = if (managed) {
require(ptr != NullPointer) { "Managed ptr is nullptr" }
Expand All @@ -39,23 +39,35 @@ actual abstract class Managed actual constructor(
}
} else null

private var __ptr: NativePointer = ptr.also {
require(it != NullPointer) {
"Can't wrap NullPointer"
}
}

actual override val _ptr: NativePointer get() = __ptr.also {
check(__ptr != NullPointer) {
"Object already closed: ${this::class.simpleName}"
}
}

actual open fun close() {
require(_ptr != NullPointer) {
"Object already closed: ${this::class.simpleName}, _ptr=$_ptr"
check(__ptr != NullPointer) {
"Object already closed: ${this::class.simpleName}"
}
requireNotNull(thunk) {
"Object is not managed in K/N runtime, can't close(): ${this::class.simpleName}, _ptr=$_ptr"
checkNotNull(thunk) {
"Object is not managed in K/N runtime, can't close(): ${this::class.simpleName}, _ptr=$__ptr"
}
require(thunk.isActive) {
"Object is closed already, can't close(): ${this::class.simpleName}, _ptr=$_ptr"
check(thunk.isActive) {
"Object is closed already, can't close(): ${this::class.simpleName}, _ptr=$__ptr"
}

thunk.clean()
_ptr = NullPointer
__ptr = NullPointer
}

actual open val isClosed: Boolean
get() = _ptr == NullPointer
get() = __ptr == NullPointer
}

@ExternalSymbolName("org_jetbrains_skia_impl_Managed__invokeFinalizer")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import kotlinx.cinterop.*
import org.jetbrains.skia.ExternalSymbolName
import kotlin.native.internal.NativePtr

actual abstract class Native actual constructor(ptr: NativePointer) {
internal actual var _ptr: NativePointer
actual abstract class Native {
internal actual open val _ptr: NativePointer

override fun equals(other: Any?): Boolean {
if (this === other) return true
Expand Down Expand Up @@ -40,11 +40,6 @@ actual abstract class Native actual constructor(ptr: NativePointer) {
actual override fun toString(): String {
return this::class.simpleName + "(_ptr=0x" + _ptr.toString() + ")"
}

init {
if (ptr == NativePtr.NULL) throw RuntimeException("Can't wrap nullptr")
_ptr = ptr
}
}

actual typealias NativePointer = NativePtr
Expand Down