Skip to content

Test that NativeFinalizer is actually invoked. #635

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
Tracked by #1031
mahesh-hegde opened this issue Apr 22, 2023 · 4 comments
Open
Tracked by #1031

Test that NativeFinalizer is actually invoked. #635

mahesh-hegde opened this issue Apr 22, 2023 · 4 comments

Comments

@mahesh-hegde
Copy link
Contributor

This was originally a checkbox under #669 . But I think this deserves separate issue.

Have to verify the NativeFinalizer is actually invoked when GC is invoked.

We should either

  • Mock the finalizer in this test
    • How?
  • Associate an atomic counter or keep some other metric in dartjni.so

https://github.com/dart-lang/ffigen/blob/master/test/native_objc_test/automated_ref_count_test.dart is the test Liam mentioned in a previous discussion but I don't think this counter-decrease-on-delete pattern is applicable in Java.

  • On Android, Dart_ExecuteInternalCommand symbol does not exist
    • Is there another way to trigger GC on Android?

cc:
@liamappelbe
@dcharkes

@dcharkes
Copy link
Collaborator

The only way that I know of without Dart_ExecuteInternalCommand is to spawn a new isolate group, attach a NativeFinalizer to a Finalizable there, and shutdown that isolate group. And then to to check that the finalizer ran, increment some integer in a memory location by one when running the native finalizer.

Though, you shouldn't have to test that native finalizers work. We have tests in the Dart SDK for that.

Have to verify the NativeFinalizer is actually invoked when GC is invoked.

Note that Dart is a generational garbage collector with an old a and new space.

https://medium.com/flutter/flutter-dont-fear-the-garbage-collector-d69b3ff1ca30

So there are both old and new space garbage collection cycles, and if an object lives in the other space than the one being collected, it will not be collected. (Also there are some cases in which the object in old space is referred to from new space garbage, in that case even an old space collection doesn't collect the object, it will only do so after the new-space garbage has been collected or promoted to old space itself.) TL;DR: at some point garbage will be collected, but no hard guarantees.

@mahesh-hegde what would you actually want to test? Do we want to test that we clean up garbage in general? E.g. that we are not forgetting to clean up.

Maybe we should have an atomic counter around creating and deleting global references, and see that that number goes down again in a longer running test?

@mahesh-hegde
Copy link
Contributor Author

what would you actually want to test? Do we want to test that we clean up garbage in general? E.g. that we are not forgetting to clean up.

Yes.

Maybe we should have an atomic counter around creating and deleting global references, and see that that number goes down again in a longer running test?

Is this reliable in a test environment? There will be other tests running, we wouldn't have much time to do the test, and this would ultimately depend on GC implementation details?

@dcharkes
Copy link
Collaborator

I was thinking the atomic counter could also be useful for ensuring that we don't hit the 51200 global refs.

I'd probably not make it part of the normal test suite, but some separate longer running script.

Yeah, it would depend on when the GC decides to run.

Maybe a better test is to make a longer running script that will most definitely fail if the native finalizers are not run. For example allocating global refs in a loop and forgetting about them. And then doing that more than 51200 times. 🙊 But trying to write the script in such a way that you also generate enough Dart objects that need to be collected.

Testing these things are hard, we know that finalizers themselves work, that's been thoroughly covered in the SDK test suites. So maybe we need to just rely on code-review that we attach finalizers to global refs.

@mahesh-hegde
Copy link
Contributor Author

It's a good idea to add a metric interface, and long running separate test.

From an organization perspective, let's detach this from #669 for now. In #669 I will assume NativeFinalizer will run and verify more boring cases.

@HosseinYousefi HosseinYousefi transferred this issue from dart-archive/jnigen Nov 17, 2023
@HosseinYousefi HosseinYousefi moved this to Backlog in JNIgen tracker Apr 11, 2024
@HosseinYousefi HosseinYousefi added this to the JNI / JNIgen 0.15.0 milestone May 22, 2024
@HosseinYousefi HosseinYousefi moved this from Backlog to Todo in JNIgen tracker May 22, 2024
@HosseinYousefi HosseinYousefi self-assigned this May 22, 2024
@HosseinYousefi HosseinYousefi removed this from the JNI / JNIgen 0.14.0 milestone Oct 23, 2024
@HosseinYousefi HosseinYousefi moved this from Todo to Backlog in JNIgen tracker Oct 23, 2024
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