This differential exposes GlobalDbSingleton API to Java native code through JNI. I understand that this differential is quite big, but all that actually matters is the .cpp file. The rest is just an API definition that is implemented in .cpp file.
Details
- Reviewers
atul • jon tomek - Commits
- rCOMMe5855464dab2: Implement JNI Helper for GlobalDBSingleton
At this moment any Java lambda that takes no arguments and returns nothing (void()) can be scheduled on the database thread and executed on the C++ side.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
native/android/app/src/cpp/GlobalDBSingletonJNIHelper.cpp | ||
---|---|---|
8 ↗ | (On Diff #17107) | Without this line task is a local reference that will be cleaned when this method completes on Java side. If we schedule a lambda to be executed at different thread and return to Java this lambda will be cleaned by the time database thread starts executing it. By making it global it will remain alive until C++ destructs it. |
10–15 ↗ | (On Diff #17107) | Without those moves I was constantly getting an error "Unable to retrieve JNI environment. Is the thread attached?". I think without those moves copy constructor gets called and it requires native API that is accessible only when calling lambda in facebook::jni::ThreadScope::WithClassLoader(...). |
We should be really sure about this code. If there are any issues, it will probably cause crashes we don't know about. I encourage everybody on this diff to be extremely thorough. We should be 100% sure of every line here. We should not be guessing, or trying things that seem to work without understanding 100% of why/how they work.
native/android/app/src/cpp/GlobalDBSingletonJNIHelper.cpp | ||
---|---|---|
10–15 ↗ | (On Diff #17107) |
Could you confirm that? This sounds a little suspicious to me - why using copy constructor requires native API? Is it the case when copying other objects? Or is Runnable different from other objects? |
native/android/app/src/cpp/GlobalDBSingletonJNIHelper.cpp | ||
---|---|---|
10–15 ↗ | (On Diff #17107) |
global_ref<T> is defined in JNI as basic_strong_ref<T, GlobalReferenceAllocator> (https://github.com/facebookincubator/fbjni/blob/main/cxx/fbjni/detail/References-forward.h#L75).
In my honest opinion I do not see reasons to consider it suspicious. Runnable is a JVM reference passed to C++, so it seems understandable that its copy constructor requires native API. I also confirmed this with SWM engineer working for Expo team.
What other object do you refer to? Runnable is a reference to object instantiated in Java, so it differs from other objects used in this piece of code that were instantiated in C++ from the beginning. |
native/android/app/src/cpp/GlobalDBSingletonJNIHelper.cpp | ||
---|---|---|
10–15 ↗ | (On Diff #17107) |
global_ref<T> is defined in JNI as basic_strong_ref<T, GlobalReferenceAllocator> (https://github.com/facebookincubator/fbjni/blob/main/cxx/fbjni/detail/References-forward.h#L75). |
native/android/app/src/cpp/GlobalDBSingletonJNIHelper.cpp | ||
---|---|---|
10–15 ↗ | (On Diff #17107) | Ok, that makes sense. Thanks for this explanation! |