Skip to content

Cycles between Dart and Java memory #575

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

Open
dcharkes opened this issue Aug 23, 2023 · 16 comments
Open

Cycles between Dart and Java memory #575

dcharkes opened this issue Aug 23, 2023 · 16 comments

Comments

@dcharkes
Copy link
Collaborator

Because the Dart and Java GCs do not know about each other, we can end up in situations where we lose reference in the Dart to a Dart object A and lose reference in Java to a Java object B, but B holds on to A via a (for example via an open port if A is a closure) and A holds on to B via a JObject.

It is not clear if this issue is going to be very common, but we should keep an eye on it: We should document any patterns that we find that might result in this. Probably we can construct one with dart-archive/jnigen#369.

If it's common we should probably provide some kind of weak reference system in at least one direction. It might depend per use case if this is possible.

(A completely out of scope, but conceptually cool, solution: A unified Dart and Java heap in the style of Oilpan which unifies the C++ DOM heap with the JS heap: https://v8.dev/blog/oilpan-library)

@HosseinYousefi
Copy link
Member

I don't think this is possible actually. Everything happens based on Java global references. If a JObject is unreachable in Dart, the underlying global ref will be deleted and if the same object had been unreachable in Java as well, it will be GCd.

Can you construct an example where this can happen?

@dcharkes
Copy link
Collaborator Author

How about of we create an interface implementation, that in one of it's closures holds on to a JObject that has a field. And then we store our just created implementation into that field?

@HosseinYousefi
Copy link
Member

Right!

Foo foo;
foo.bar = Interface.implement($InterfaceImpl(
  f: () {
    return foo;
  }
));

Now when foo is lost, it's still holding .bar which holds foo.

@HosseinYousefi HosseinYousefi transferred this issue from dart-archive/jnigen Nov 17, 2023
@HosseinYousefi
Copy link
Member

Let's say in the previous example of

Foo foo;
foo.bar = Interface.implement($InterfaceImpl(
  f: () {
    return foo;
  }
));

We want to use foo, but not hold on to it. One might want to do this:

Foo foo;
foo.bar = Interface.implement($InterfaceImpl(
  f: () {
    return weak(foo);
  }
));

But the act of usingfoo means that we're holding on to it. Instead we can "capture" all weak references beforehand, but this is manual and it's hard to come up with any nice syntax to do this.

One could wrap the callback to f with a function like capture that gets a bunch of JObjects and the original callback (in addition to the captured variables). But in order to preserve the argument types and captured types, we need O(n*m) different capture functions, n being the maximum number of args for a function we want to support and m being the maximum number of captured variables we want to support. Like this for n=1 and m=1:

T Function(A1) captureWeak1Thing1<T, A1, C1 extends JObject>(
    C1 c1, T Function(A1 arg0, {required C1 $1}) callback) {
  return (A1 a1) => callback(a1, $1: c1.toWeak());
}

@dcharkes
Copy link
Collaborator Author

What is Dart and Java objects in this example?

Foo is a JNI generated class? foo a JNIReference. The Dart closure stays alive because foo.bar in Java holds on strongly to the Dart closure (a Dart_PersistentHandle under the hood). And the closure keeps foo in Java alive because its a JNI global handle.

I was thinking if we could use Java's WeakReference. But that doesn't work. We don't want the interface implementation closure to be only weakly referenced from Java, it would just immediately be GCed.

I was thinking if we could use Dart's weak reference on foo in the closure:

void something(Foo foo) {
  WeakReference<Foo> weak = WeakReference(foo);
  foo.bar = Interface.implement($InterfaceImpl(
    f: () {
      final target = _cache?.target;
      if (target == null) {
        // throw exception in Dart that causes exception in Java
        // or can't we do that and do we need something in the JNI code generation to throw the Java exception in C code?
      }
      return target;
    }
  ));
}

@HosseinYousefi Would this work?

@HosseinYousefi
Copy link
Member

Your assumptions are correct.

I was thinking if we could use Dart's weak reference on foo in the closure:

That's exactly what I'm trying to do. However one has to manually turn the objects into weakly held ones as you did in your code sample before using them in the callback. I'm trying to come up with an easier API.

// or can't we do that and do we need something in the JNI code generation to throw the Java exception in C code?

We can do that already, it throws a DartException but we could allow throwing any Throwable as well (#561).

@dcharkes
Copy link
Collaborator Author

That's exactly what I'm trying to do. However one has to manually turn the objects into weakly held ones as you did in your code sample before using them in the callback. I'm trying to come up with an easier API.

Hm, that does mean adding more magic to JNIgen, instead of off-the-shelf Dart features. I'd have to be convinced it's worth the complexity. Do you have a compelling example in which we don't want to just teach JNIgen users to use Dart's WeakReference?

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Mar 17, 2024

That's exactly what I'm trying to do. However one has to manually turn the objects into weakly held ones as you did in your code sample before using them in the callback. I'm trying to come up with an easier API.

Hm, that does mean adding more magic to JNIgen, instead of off-the-shelf Dart features. I'd have to be convinced it's worth the complexity. Do you have a compelling example in which we don't want to just teach JNIgen users to use Dart's WeakReference?

Oh, so you mean using the existing WeakReference, instead of JNI's weak global reference. That could work as well, I think it's cleaner to use weak global reference though:

void something(Foo foo) {
  final weakFoo = foo.toWeak(); // This will create another Foo object that has an internal JWeakGlobalReference instead.
  foo.bar = Interface.implement($InterfaceImpl(
    f: () {
      if (weakFoo.isNull) {
        // throw exception in Dart that causes exception in Java
      }
      return weakFoo;
    }
  ));
}

@dcharkes
Copy link
Collaborator Author

Hm, that could indeed work as well.

I'm not entirely sure which of the two approaches is better. Some thoughts:

  • If we have a reference somewhere else in the program, having also a weak reference means we have two JNI references floating around (or O(N) references if we have N capturing closures). I'm not sure how conservative we should be on trying to create the least amount of JNI references or not.
  • If we lose the strong reference in Dart, but it's not eagerly freed, then it requires a GC in Dart before a GC in Java before the thing that we weakly reference gets GCed in Java, no matter which of the two approaches we use. If we use a JNI weak reference as you suggest, and the strong reference is eagerly cleaned up, then only a Java GC is needed. So this points in favor of using JNI weak refs. (How common is it that users eagerly release?)
  • Seeing if the weak-ref still has a value might be quicker if we use a Dart weak ref, it doesn't require going through FFI. So performance might be slightly better with the Dart weak ref approach.
  • It feels weird to not expose the JNI weak ref, we're building a package:jni.
  • If we expose the JNI ref, users have two ways of doing things. (We can add documentation to discourage users using Dart weak refs if we expose JNI weak refs to mitigate this.)

@dcharkes
Copy link
Collaborator Author

dcharkes commented Mar 21, 2024

Notes from discussion:

  • We probably don't want to put JWeakReference next to JGlobalReference and have JObject have possibly a weak reference backing it. Reasons: It's cleaner to call a .target first and have a strong reference and then do multiple interactions.
  • If we expose JNI weak references, it should probably be JWeak<T extends JObject> with a single getter .target.
  • We're not sure if we should implement that or just rely on Dart WeakReferences for now.

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Feb 11, 2025

Writing some docs about this at the moment. Using weak references converts

graph TD;
   f-->|holds in Dart|foo;
   foo-->|holds in Java|bar;
   bar-->|holds in Java|f;
Loading

into

graph TD;
   f-.->|holds weakly in Dart|foo;
   foo-->|holds in Java|bar;
   bar-->|holds in Java|f;
Loading

Here's a sample code:

final weakFoo = WeakReference(foo);
foo.bar = Bar.implement($Bar(
  f: () {
    final foo = weakFoo.target;
    if (foo == null) {
      throw StateError();
    }
    return foo;
  }
));

However, we still have to be careful that foo itself should not be released in Dart. The object foo might used/stored in Java and not released, however the way we access it through Dart is through weakFoo which is a weak reference to foo.

This means that if we do foo.release(), the resulting foo is invalid due (use after release). We should have a better guideline on using other JObjects inside interface implementation than simply using weak references.


Here's a possible design. Let's have Bar.implement get a map of String -> JObject which are the "capture group". Now the references of all these "captured" objects will be passed to Java and whenever foo.bar is GC'd, all of the capture group will also be released. To access object in the capture group, you can use the provided context object:

foo.bar = Bar.implement($Bar(
  $capture: ['foo': foo], 
  f: (context) { // All methods get an additional context
    final foo = context['foo']!.as(Foo.type);
    return foo;
  }
));

operator [] of context does a JNI call to fetch the object corresponding with the key foo from Java, so the cycle is created completely in Java which is fine.

@dcharkes
Copy link
Collaborator Author

I'm not sure I understand how that would change the example. Doesn't that simply create a strong reference cycle between foo and bar?

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Feb 11, 2025

I'm not sure I understand how that would change the example. Doesn't that simply create a strong reference cycle between foo and bar?

It does, but in Java which is fine.

The way we can implement this is we send the entire map to be stored in Java, the map itself is no longer referenced in the closures, instead we access the elements through the context argument which overloads operator [] to fetch objects from Java. It's basically as if we're implementing an interface in Java, sure it creates a strong reference, and actually we want a strong reference. We just don't want the cycle to be between the two GCs.

@dcharkes
Copy link
Collaborator Author

Right, foo is already a Java object. We implement a callback of a Java interface with Dart, and in there we want to access a Java object. But instead of having the Dart closure hold on to the Java object, you have the Java closure hold on to the Java object. And to get back that Java object in Dart code we provide a method context[], which is implemented as a Java method call that gives us back a reference to that object.

You want to borrow explicit "capture"s from C++ to signify this difference in behavior. I'm not sure if capture is the right word here, because you're capturing either in Dart or in Java (or in both, if you're not careful). But the doc-comment for the capture and context could explain this.

So, the only things that should be captured are JObjects. It's kinda like "unwrap-capture" or "deref-capture". (And we'd want lints that give you a hint if you accidentally capture a JObject in the Dart closure?)

foo = context['foo']!;

Doesn't this require a as SomeSubtypeOfJObject?
Also, to make abundantly clear there is no capture in the Dart closure, it would probably be cleaner if it would be a new declaration final foo = ...;.

More unwrapping/dereffing musings

I was trying to come up with a workaround without generating this code pattern. But, one can't manually make the context in Java as a map and pass a closure in that does a lookup in that map. Because the Dart closure would hold on to that method, which holds on to those objects. So this pattern actually needs to be part of how we do interface implementations.

This does create an issue with if you want to apply the same pattern for normal callback functions. You can't easily pass a JReference to some Kotlin API that wants a callback. Because we don't want to change the signature to take this extra context. How do we do the same level of unwrapping if we pass a Kotlin reference into a kotlin function that wants a callback argument?

I guess there's also a question for if you want to do an interface implementation but you don't want to actually provide all methods in Dart, but just provide a JReference to a closure for some methods. That currently forces you to go through Dart. If we could unwrap there as well, that would also be good.

@HosseinYousefi
Copy link
Member

HosseinYousefi commented Feb 11, 2025

(And we'd want lints that give you a hint if you accidentally capture a JObject in the Dart closure?)

Yes, that would be ideal.

Also, to make abundantly clear there is no capture in the Dart closure, it would probably be cleaner if it would be a new declaration final foo = ...;.

Yeah, I missed typing final :P

How do we do the same level of unwrapping if we pass a Kotlin reference into a kotlin function that wants a callback argument?

We keep having KtFunctionK.implement(...). If we have this KtFunctionK from somewhere, maybe as a result of calling another Kotlin function, then we can pass that in.

final function2 = kotlinMethodReturningAFunction2();
// both work:
kotlinMethodExpectingAFunction2(function2); 
kotlinMethodExpectingAFunction2(KtFunction2.implement(...)); 

That currently forces you to go through Dart. If we could unwrap there as well, that would also be good.

We currently have function and function$async per each function. We could have function that can be Function.sync(...), Function.async(...) and Function.fromJavaFunction(...). I don't think this pattern is used that much though.

Doesn't this require a as SomeSubtypeOfJObject?

Yes it does, fixed in the original comment.

@liamappelbe
Copy link
Contributor

Clever solution. I'm not a big fan of storing the context as a map, but I can't think of a better way. The ObjC equivalent would be:

foo.bar = Bar.implement(
  $capture: ['foo': foo], 
  f: (context) {
    final foo = Foo.castFrom(context['foo']!);
    return foo;
  }
);

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

No branches or pull requests

3 participants