Fix windows

[HosseinYousefi]

Closes dart-lang/native#592.

Also fixes the leaking of global references by cleaning them up after use in Java.

---

The fix for windows is a single line removal from dartjni.h. Windows doesn't have a function to destroy condition variable but I accidentally used the function to destroy the mutex for it. See the commit with 🤦‍♂️!

---

The rest of the PR is fixing a memory ownership problem. Instead of attaching the return value of the implemented method to a NativeFinalizer. We are passing the global reference to Java first and get an object from it. And then deleting the global reference used.

We can't do this cleanly in a single native function because if we delete the global reference, we can no longer return the result object.

Since Dart doesn't know that this global reference is still used, it might garbage collect it via NativeFinalizer thus making it invalid. So I'm letting Java handle the clean up and use setAsDeleted to detach the native finalizer from the return value in Dart.

[coveralls]

Pull Request Test Coverage Report for Build 5698374749

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.003%) to 90.289%

---

Totals
Change from base Build 5658078280:	0.003%
Covered Lines:	2966
Relevant Lines:	3285

---

💛 - Coveralls

[dcharkes]

There's no description on the PR what we're fixing (and I have trouble reverse engineering the issue from the diff 🙃).

[dcharkes]

Is this racy? assigning to a global variable?

[HosseinYousefi]

load_class_global_ref acquires a lock.

[dcharkes]

Why does calling two native methods instead of 1 make a difference? Shouldn't we be able to do in a single C function what we can do in two?

[HosseinYousefi]

Explained below.

[dcharkes]

If I understand correctly, we're boxing here. Why do we need boxing for Windows?

[HosseinYousefi]

We're returning an array of Objects so unfortunately, we have to box the long value.

[HosseinYousefi]

There's no description on the PR what we're fixing (and I have trouble reverse engineering the issue from the diff 🙃).

The fix for windows is a single line removal from dartjni.h. Windows doesn't have a function to destroy condition variable but I accidentally used the function to destroy the mutex for it. See the commit with 🤦‍♂️!

---

The rest of the PR is fixing a memory ownership problem. Instead of attaching the return value of the implemented method to a NativeFinalizer. We are passing the global reference to Java first and get an object from it. And then deleting the global reference used.

We can't do this cleanly in a single native function because if we delete the global reference, we can no longer return the result object.

Since Dart doesn't know that this global reference is still used, it might garbage collect it via NativeFinalizer thus making it invalid. So I'm letting Java handle the clean up and use setAsDeleted to detach the native finalizer from the return value in Dart.

[dcharkes]

The rest of the PR is fixing a memory ownership problem. Instead of attaching the return value of the implemented method to a NativeFinalizer. We are passing the global reference to Java first and get an object from it. And then deleting the global reference used.

We can't do this cleanly in a single native function because if we delete the global reference, we can no longer return the result object.

Since Dart doesn't know that this global reference is still used, it might garbage collect it via NativeFinalizer thus making it invalid. So I'm letting Java handle the clean up and use setAsDeleted to detach the native finalizer from the return value in Dart.

Ah, that makes sense. 👍 Please add that as some documentation in the code and on the PR.