Open
Conversation
Add all the new module files, updates the plugins with new module. The service stub is minimal empty implementation that does not yet handle any jni. It can be created, allows setting a backend, it provides empty methods common for all service adapters.
Adds gradle files necessary for the android build, adds ue4shared.keystore also needed for the android app to be run and the jni_UPL.xml which allows adding the configuration for unreal generated xml file the upl does not yet have necessary configuration
Fill in the upl file to copy necessary java modules. The modules must be placed inside the plugin directory in the android folder. This code is generated by java template. The also expose the services provided by the java code, adds permission for binding, adds queries (this part is necessary for clients). All the dependencies for includes and modules are also added.
Adds the code generated for the jni implementation for backend. It requires implementing native functions. For now they are added to service adapter but without implementation.
Introduce a service starter class that allows starting and stopping the android service provided by the java template. An android service needs to be explicitly started and in a thread of the unreal application, JNI backend with unreal implementation must be provided to it. The java part of service starter of the JNI module, handles most of that, the added cpp class runs that code. The object returned by it is the java JNI service backend that was instantiated for the service. It should be used to perform operations on it.
JNI data types must be translated into cpp data types, and vice versa. The data converter helps convert structures and enums added in the modules. It also handles the arrays of those types.
execute the jni service backend function informing about ready state changed.
Adds a handle for the unreal service class this way the JNI implementation can access necessary, public functions of the unreal service class. This will be used for e.g. handling property change requests.
Add data translation from cpp type to java type, that may be reused later. Provide translated property to jni.
The function purpose is to inform the jni implementation about new value, whenever the value of the service was changed. The "In" prefix was removed from property name to be able to reuse the common code for translating cpp to java types.
The purpose of this function is to inform the JNI implementation that the service emitted signal. Data is translated for all parameters and JNI is informed about the signal with correct parameters.
Added the data translation from java to cpp. This was not templated. The function must be executed with AsyncTask, this is because request comes from other than game thread, but results in broadcasting delegate and some subscribers are expected to be in blueprints.
Translates data from java to cpp and result from cpp to java. Does not use template for the result to keep the name more meaningful. WARNING: the executing of method may cause some side effects like changing the property or emitting a signal. In this case the application will crash. The way to solve this is either always broadcast with AsyncTask or have a promise for the result, schedule executing method with AsyncTask which should fill the result. The method should wait for it.
The tests_common are missing usecase for array of enums This is not essential but makes the testing easier.
Minimal non functional implementation of JNI client. Implements the UAbstract interface, but returns only default values. Adds the JNI client java files that requires implementing the native methods. The implementation of them just returns default values.
Make the instance of java jni client. Add functionality to bind and unbind to a service through the jni client.
When the jni client notifies it is ready to use (or changes that status to false) it calls native function. This notification should be broadcasted to whoever is interested about the connection status changed. For this purpose the delegates are added and exposed. The commit adds those delegates along with a dummy class which is necessary for the delegates to properly compile. The commit introduces a mechanism of global function, that is provided by the unreal client impl on init and set to an empty default version on deinit. Thanks to that function a more complexed action may be executed like allow changing a member value (to track the state). Also AsyncTask is used as the broadcasting informs also subscribers that are expected to be in blueprints.
The purpose of this function is pass the request of property change to a java jni client side (which later will be sent to an android service). Introduces the template for data translation from cpp to java param. The template uses cpp name with "In" prefix.
Introduces a global function for each property that gets changed. The function by default just logs, but is set but the unreal client on init and reset to the empty one on deinit. The behavior added by unreal client is: to change the local value and broadcast the value. The fact that it needs change the object state makes it impossible to just use the public api of the object.
It uses the global pointer to the unreal client class, which is set in the init and reset in deinit. Since the signals are only to be broadcasted and no internal state needs to be changed it was enough just to expose the unreal client, no need for extra functions.
Executing a methods requires preparing a promise and storing it while waiting for the message reaches the service and a response is delivered back to the client. This commit provides a method helper which allow storing promises for certain result types. This object is stored globally (the client is anyway a subsystem).
Forwards the method execution request to jni client, along with a unique id. Handles the native function which provides the result, and based on the id resolves proper stored promise.
Most probably should be updated for ue5.6 version.
When executing an operation through the JNI Client, we're calling an async java method. Since the method is asynchronous, we use a TPromise and TFuture on the UnrealEngine side. In the synchronous operation execution, we're blocking until the TFuture is completed in the calling thread. This means, that the calling thread (most likely the GameThread is blocked until the TFuture is resolved). The TFuture<Result> associated with operation call will be resolved once the result for the operation is returned via *_nativeOn<Operation>Result. However, in current implementation, the MethodHandler::FulfillPromise issues an async task to the GameThread, which sets value on the promise (resolves the future). This is problematic, since the GameThread may be blocked on the TFuture::Get() call, causing a deadlock. Since the calling thread (most likely GameThread) stops the execution until future is resolved, it is safe to set value on it in a thread other than GameThread.
Although this situation is extremely rare, and in fact, it should never happen, if class is set, while operations java async method id isn't we will hang forever due to awaiting a never fulfilled promise. Make sure the promise is fulfilled with default value when method id is null.
In every sync operation, which returns a result, we create a TPromise,
even if the code isn't compiled for android with JNI support.
Since unfulfilled TPromises cause runtime crashes when deleted, our
design has two issues:
1. During class validity checking, we're returning as it was a sync
method, which doesn't fulfill the created process - this will cause
a crash.
2. When our code isn't compiled for android with jni support the
TPromise if never fulfilled either. This will also cause a crash.
Prevent both issues by moving the declaration of TPromise to code, which
is compiled only for android with JNI support, after checking for the
validity of java class.
If the code isn't compiled for android with JNI support, simply return
(optionally the default value).
We want to support async operations in JNI client. In case of a sync method, the (scope of) operation itself may be the owner of the TPromise associated with the async java call. However, this is no longer possible in case of an async method, where the TPromise outlives the scope of method. Therefore, we're going to promote the MethodHelper to be the single owner of async java call promises. This implies, that the caller of MethodHelper::StorePromise must move the promise to StorePromise function, instead of passing it by a reference. In current implementation, this would cause a Use-After-Move error. Therefore, as a preparatory change, get the future associated with the promise earlier, just after creating the promise.
Currently, we're locking a critical section during MethodHandler::StorePromise for the whole scope of the method. Since we're doing much more work, than simply adding the promise to a map, including logging a message, we introduce a very slight chance of a deadlock. Prevent any possible issues by locking the critical section only for the relevant operation, that is, adding the promise to the map.
This is a preparatory change to extracting the MethodHelper class.
MethodHelper class is redeclared in each JNI client. That is, every defined interface will have a different class defined. Reduce the amount of duplicated code by extracting MethodHelper to a module scoped header. This let's us generate only one MethodHelper class per module. Introduce a Detail subdirectory in the Private/Generated folder, which contains implementation details of the module. Place the class in this directory.
We want to support async calls in JNI client. However, when issuing a java async call, we create a TPromise, and calling code awaits for the associated TFuture to be completed. Until now, java async calls were only used by the JNI client's sync calls, where the sync method was awaiting for the completion of the TFuture, keeping said TPromise alive until async java call returns result. In async operation version, the associated TPromise will outlive the scope of the method. We need to guarantee, that java async call result, which comes from JNI thread, can safely access and resolve the associated TPromise. To ensure consistent TPromise lifetime handling across synchronous and asynchronous calls, update the design and responsibilities of MethodHelper. Make MethodHelper the exclusive owner of promises for Java asynchronous calls, and store all associated TPromise instances there so it manages their lifetimes. Introduce a virtual (T<Module>PromiseHolder) wrapper for TPromises, which ensure proper handling of all TPromises with TUniquePtr. Prevent crashes due to unfulfilled promises by fulfilling all promises with default values just before the client is deinitialized. This is the safest way to deal with possibly unfulfilled promises. If we don't do it, we risk crashing the engine during shutdown, which we don't want. The issue here is, that we don't know, what will happen with TFutures given to the callers of async methods, but theres nothing we can do at this point.
We want to support async variant of the operations. The code will be very similar to the existing sync operation. Extract the shareable part to a separate function.
When calling an async java method, a JNI error can occur. Currently, we check it and ignore the information. Although a JNI exception during execution of async java method should be rare, it may cause a deadlock of the calling thread (presumably the GameThread). Assume, that we won't receive a result for the operation, if we receive a JNI error during execution of async java call (tryCallAsyncJava<Operation>). In such situation, fulfill the promise to prevent a deadlock.
Until now, we supported only sync operations, even though they used async calls underneath. Implement the async operations in JNI client. Use the extracted function to call async java methods.
Currently, reading and writing properties on the default (stub) implementation is only safe when done from a single thread. Getting or Setting property values using Set<Property> or Get<Property> from different threads will cause data races, where the actual value is unknown. When using the JNI Adapter, this becomes an issue, since Get<Property> and Set<Property> requests come from JNI thread. Native java implementations call Get<Property> and Set<Property> directly on a service from calling thread (JNI thread), which requires the implementation of backends Set<Property> and Get<Property> to be thread safe. Make Set<Property> and Get<Property> functions thread safe when using the stub implementation. Important: a developer, who creates a custom backend must ensure, that Setting and Getting properties is thread safe when he wants to use the backend with JNI Adapter.
Currently, we're running operations on the interface object from the JNI
thread. Although accessing the interface object is safe, calling the
operations on it in non GameThread is very risky.
We're basically passing the responsibility of thread safety management
to a developer. This is very error prone solution, as
1. The developer must understand the risks associated with
mutlithreading.
2. The developer must understand, that JNI uses multiple threads under
the hood (in the client and adapter), which could be considered as
"library code" - the code, that a developer shouldn't need to
modify)
Our implementation details, such as receiving operation calls from JNI
in a special thread, shouldn't be exposed outside of our
implementations.
The call to operation on a service should be done on the GameThread -
the thread, where the object lives to decrease the possibility of data
races and usage of UObjects outside of the GameThread.
Introduce a ThreadingHelper, providing two functions:
1. RunInGameThreadAndWait
2. EvalInGameThread
... where the first one runs a task on the GameThread, and the second
one evaluates a value on the GameThread.
Both methods suspend the execution of calling thread until the task /
evaluation finishes, therefore it's safe to capture values by reference
in the calling thread.
One must prevent keeping any lock in the calling thread when calling
either of these functions to prevent deadlocks.
It's safe to call these functions from the GameThread - the task will be
executed directly.
Use the introduced functions when implementing operation handling in JNI
adapter.
Fix typo in CheckJniErrorOccurred and make it start with uppercase. This is a preparatory change to move the function to other class.
Extract checkJniErrorOccurred to a CommonJavaConverter class, which is accessible from all interfaces inside the module. Rename the method to CheckJniErrorOccurred. Leave the checkJniErrorOccurred function in each interface converter, but use the extracted function inside.
Currently, we're removing references to arrays, after casting. However, this is incorrect, since casting a java object doesn't automatically make it a local reference. This makes us delete a local reference, which wasn't created by us, which is a bug. Prevent deleting references, which were given to us.
Extract conversions of jstring arrays to TArray<FString> and j<primitive>arrays to TArray<Primitive> to a specialized class using template methods. This simplifies the template code and moves the conversions from 3 places in the template (multiple places in the generated code) to just a single point per module. This is a preparatory change to handle null values received from JNI.
Any Java object can be null. We use null as the default value for java strings, enums, structures, interfaces and externs. However, we currently don't handle correctly receiving null values from JNI. Instead, we always assume, the object is valid (isn't null). This is clearly a bug. Although, calling java methods on null objects shouldn't crash Unreal Engine application, it will certainly crash the JVM, which isn't acceptable. Prevent attempts to convert null values in ApiGear converters. It's not necessary in case of FJavaHelper::FStringFromParam, since it check the validity of object internally.
Currently, we generate the Build.cs file on the Module scope, and not on interface scope, like in other communication protocols (olink, monitor, msgbus). Because of this, ue4 gets the following error while building: ERROR: Unable to determine UHT module type for ... ... for modules without interfaces, for example the CustomTypes module of goldenmaster. Fix this, by moving the generation of Build.cs file to the interface scope.
This commit should not exist. In fact, these 2 commits shouldn't exist either: 6883823 fix(jni): register jni module conditionally e8bbeba fix(jni): script build fix However, the build configuration for jni differs from other communication features (olink, msgbus) and needs to be documented in form of a separate commit. The jni feature provides data converters for custom types defined in a module. It defines them on the module scope. These converters must be built. Otherwise, we lose the converters for these types - if any other module depends on these types, the compilation will fail.
On UE < 5.7, BuildPlugin -Rocket -Package omits the JNI module entry
from the packaged uplugin when building imported modules (e.g.
CustomTypes, ExternTypes) — because the module produces no output for
non-Android targets. The Build.cs is still included in the package
source, so when a dependent module (e.g. Counter) builds against the
installed plugin, UHT finds the Build.cs but cannot locate a matching
uplugin entry → "Unable to determine UHT module type" error.
Fix by making JNI-specific code conditional on the target platform:
- Wrap imported module includes in datajavaconverter.h/.cpp.tpl,
jniadapter.cpp.tpl behind #if PLATFORM_ANDROID
- Add explicit #include "HAL/Platform.h" in datajavaconverter.h.tpl,
jniclient.h.tpl, and jniadapter.h.tpl to ensure PLATFORM_ANDROID is
defined (avoids -Wundef on strict compilers)
- Make imported Jni module deps in modulejni.Build.cs.tpl conditional
on Target.Platform == Android
JNI modules remain visible to the Editor on all platforms; only the
Android-specific includes and dependencies are gated.
alwa-siili
force-pushed
the
feat/add-android
branch
from
72afd0d to
84c0b4b
Compare
alwa-siili
changed the title
The ue4shared.keystore was added by mistake in 7b895d2 and is never referenced by any code: not in rules.yaml, gradle signing configs, UPL XML, or source files. UE handles APK signing automatically. Debug builds use Gradle's built-in ~/.android/debug.keystore, and distribution builds require the developer to configure their own keystore via Project Settings. Plugins have no role in APK signing, and shipping a shared keystore is a security anti-pattern.
alwa-siili
force-pushed
the
feat/add-android
branch
2 times, most recently
from
335e975 to
a32ca6a
Compare
Add nativeAsyncOperationFailed JNI callback to each interface client. When Java reports a failed async operation, log a warning with error details and fulfill the pending promise with a default value. Add FullfillPromiseWithDefaultValue to MethodHelper. Look up the promise by GUID, remove it from the map, and call FulfillWithDefaultValue on the IPromiseHolder base. Without this, a failed Java async operation leaves its TPromise unfulfilled, deadlocking any game-thread code that calls TFuture::Get().
alwa-siili
force-pushed
the
feat/add-android
branch
from
a32ca6a to
3842841
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
📑 Description
✅ Checks
ℹ Additional Information