Skip to content

Commit

Permalink
Fix Subclassed WeakReference to be GCed (scala-native#3347)
Browse files Browse the repository at this point in the history
Instead of using WeakReferenceId directly to determine WeakReference, we use the ranges of WeakReference to make the determination, as `Class.is` does.

(cherry picked from commit 8c87a9a)
  • Loading branch information
mox692 authored and WojciechMazur committed Sep 1, 2023
1 parent fe96cb9 commit 46c6b8f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
#include "GCTypes.h"

extern int __object_array_id;
extern int __weak_ref_id;
extern int __weak_ref_ids_min;
extern int __weak_ref_ids_max;
extern int __weak_ref_field_offset;
extern int __array_ids_min;
extern int __array_ids_max;
Expand Down Expand Up @@ -68,7 +69,8 @@ static inline size_t Object_Size(Object *object) {
}

static inline bool Object_IsWeakReference(Object *object) {
return object->rtti->rt.id == __weak_ref_id;
int32_t id = object->rtti->rt.id;
return __weak_ref_ids_min <= id && id <= __weak_ref_ids_max;
}

static inline bool Object_IsReferantOfWeakReference(Object *object,
Expand Down
16 changes: 11 additions & 5 deletions tools/src/main/scala/scala/scalanative/codegen/Generate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -437,10 +437,10 @@ object Generate {
Val.Int(value)
)

val (weakRefId, modifiedFieldOffset) = linked.infos
val (weakRefIdsMin, weakRefIdsMax, modifiedFieldOffset) = linked.infos
.get(Global.Top("java.lang.ref.WeakReference"))
.collect { case cls: Class if cls.allocated => cls }
.fold((-1, -1)) { weakRef =>
.fold((-1, -1, -1)) { weakRef =>
// if WeakReferences are being compiled and therefore supported
val gcModifiedFieldIndexes: Seq[Int] =
meta.layout(weakRef).entries.zipWithIndex.collect {
Expand All @@ -454,9 +454,14 @@ object Generate {
"Exactly one field should have the \"_gc_modified_\" modifier in java.lang.ref.WeakReference"
)

(meta.ids(weakRef), gcModifiedFieldIndexes.head)
(
meta.ranges(weakRef).start,
meta.ranges(weakRef).end,
gcModifiedFieldIndexes.head
)
}
addToBuf(weakRefIdName, weakRefId)
addToBuf(weakRefIdsMaxName, weakRefIdsMax)
addToBuf(weakRefIdsMinName, weakRefIdsMin)
addToBuf(weakRefFieldOffsetName, modifiedFieldOffset)
}

Expand Down Expand Up @@ -558,7 +563,8 @@ object Generate {
val moduleArrayName = extern("__modules")
val moduleArraySizeName = extern("__modules_size")
val objectArrayIdName = extern("__object_array_id")
val weakRefIdName = extern("__weak_ref_id")
val weakRefIdsMaxName = extern("__weak_ref_ids_max")
val weakRefIdsMinName = extern("__weak_ref_ids_min")
val weakRefFieldOffsetName = extern("__weak_ref_field_offset")
val registryOffsetName = extern("__weak_ref_registry_module_offset")
val registryFieldOffsetName = extern("__weak_ref_registry_field_offset")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import org.scalanative.testsuite.utils.Platform
class WeakReferenceTest {

case class A()
class SubclassedWeakRef[A](a: A, referenceQueue: ReferenceQueue[A])
extends WeakReference[A](a, referenceQueue)

def gcAssumption(): Unit = {
assumeTrue(
Expand All @@ -36,6 +38,16 @@ class WeakReferenceTest {
weakRef
}

@noinline def allocSubclassedWeakRef(
referenceQueue: ReferenceQueue[A]
): SubclassedWeakRef[A] = {
var a = A()
val weakRef = new SubclassedWeakRef(a, referenceQueue)
assertEquals("get() should return object reference", weakRef.get(), A())
a = null
weakRef
}

@deprecated @nooptimize @Test def addsToReferenceQueueAfterGC(): Unit = {
assumeFalse(
"In the CI Scala 3 sometimes SN fails to clean weak references in some build configurations",
Expand Down Expand Up @@ -71,22 +83,33 @@ class WeakReferenceTest {
val refQueue = new ReferenceQueue[A]()
val weakRef1 = allocWeakRef(refQueue)
val weakRef2 = allocWeakRef(refQueue)
val weakRefList = List(weakRef1, weakRef2)
val weakRef3 = allocSubclassedWeakRef(refQueue)
val weakRefList = List(weakRef1, weakRef2, weakRef3)

GC.collect()
def newDeadline() = System.currentTimeMillis() + 60 * 1000
assertEventuallyIsCollected("weakRef1", weakRef1, deadline = newDeadline())
assertEventuallyIsCollected("weakRef2", weakRef2, deadline = newDeadline())
assertEventuallyIsCollected("weakRef3", weakRef3, deadline = newDeadline())

assertEquals("weakRef1", null, weakRef1.get())
assertEquals("weakRef2", null, weakRef2.get())
assertEquals("weakRef3", null, weakRef3.get())
val a = refQueue.poll()
assertNotNull("a was null", a)
val b = refQueue.poll()
assertNotNull("b was null", b)
val c = refQueue.poll()
assertNotNull("c was null", c)
assertTrue("!contains a", weakRefList.contains(a))
assertTrue("!contains b", weakRefList.contains(b))
assertNotEquals(a, b)
assertTrue("!contains c", weakRefList.contains(c))
def allDistinct(list: List[_]): Unit = list match {
case head :: next =>
next.foreach(assertNotEquals(_, head)); allDistinct(next)
case Nil => ()
}
allDistinct(List(a, b, c))
assertEquals("pool not null", null, refQueue.poll())
}

Expand Down

0 comments on commit 46c6b8f

Please sign in to comment.