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

WeakReference does not work if subclassed #3341

Closed
armanbilge opened this issue Jun 17, 2023 · 2 comments · Fixed by #3347
Closed

WeakReference does not work if subclassed #3341

armanbilge opened this issue Jun 17, 2023 · 2 comments · Fixed by #3347

Comments

@armanbilge
Copy link
Member

It seems that if WeakReference is sub-classed then it is not correctly handled by the GC. Probably due to the logic here?

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

Consider the following program, which allocates large arrays and stores them in weak references. It terminates when it detects the that a weak reference has been GCed.

import java.lang.ref.WeakReference

class MyWeakRef[A](a: A) extends WeakReference(a)

@main def main =
  val refs = new Array[MyWeakRef[Array[Long]]](Int.MaxValue / 8)

  var i = 0
  while (i < refs.length) {
    refs(i) = new MyWeakRef(new Array[Long](Int.MaxValue / 64))
    var j = 0
    while (j < i) {
      if (refs(j).get == null) {
        println((i, j))
        System.exit(0)
      }
      j += 1
    }
    i += 1
  }

As written, that program never prints or terminates for me. This suggests that none of the arrays are being GCed.

However, if I replace MyWeakRef with WeakReference then it terminates quickly with (3,0). If I run the program on the JVM, it also terminates quickly.

Motivation

In Cats Effect we sub-class WeakReference for a custom data structure. We use this data structure to power a "fiber dump" feature (similar to thread dump).

https://github.com/typelevel/cats-effect/blob/e9aeb8ccb53016a37588256f4c8086f8ef1612bc/core/shared/src/main/scala/cats/effect/unsafe/WeakBag.scala#L108-L110

@WojciechMazur
Copy link
Contributor

WojciechMazur commented Jun 19, 2023

We should replace current direct comparison of class id with a port of Class.is(Class[_], Class[_]) https://github.com/scala-native/scala-native/blob/main/nativelib/src/main/scala/java/lang/Class.scala#L74-L96

For the WeakReference detection we can use simplified algorithm, because WeakReference is a class, and trait would never be instantiated, we might get:

  val WeakReferenceId = ???
  private def isWeakReference(cls: _Class[_]): Boolean =
    cls.id >= WeakReferenceId && cls.idRangeUnti <= WeakReferenceId

This should be translated to C using ObjectHeader.h datastructres https://github.com/scala-native/scala-native/blob/main/nativelib/src/main/resources/scala-native/gc/immix_commix/headers/ObjectHeader.h#L28-L42

@mox692
Copy link
Contributor

mox692 commented Jun 19, 2023

I will work on this.
To me, it looks good issue to delve into scala-native internals and gc stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants