Return Application AssetManager from AndroidApp::asset_manager#233
Merged
Conversation
rib
force-pushed
the
rib/pr/sound-application-asset-manager
branch
from
98c8bcf to
abe08f7
Compare
rib
force-pushed
the
rib/pr/run-on-java-main-thread
branch
2 times, most recently
from
a087094 to
49bbf6c
Compare
rib
force-pushed
the
rib/pr/sound-application-asset-manager
branch
2 times, most recently
from
8806f7c to
d562cc5
Compare
This makes sure that the `AssetManager` we return from `AndroidApp::asset_manager` can be retained with a static lifetime and never become a wrapper for an invalid pointer. The key change here is that we now return the Application AssetManager (i.e. from Application.getAssets()) instead of the Activity AssetManager. Theoretically there could be some applications that could associate an Activity AssetManager with unique resources but that's not expected to be common (and at least no expected to affect anyone currently using `AndroidApp::asset_manager`). As part of the `APP_ONCE` initialization in `init_android_main_thread` we now get a global reference to the Application AssetManager and get the corresponding AAssetManager that we can trust will be valid for the lifetime of the process since we leak the global reference. Note: The Application `AssetManager` is logically a process-wide resource and so the leaked global is just a technical formality to ensure it can't be garbage collected, but that's assumed to be redundant. Note: If anyone _strictly_ needs the `Activity` `AssetManager` then they could at least resort to calling `Activity.getAssets()` via JNI manually, but perhaps we can later consider adding a separate `AndroidApp::activity_asset_manager()` that will pair an `AAssetManager` pointer with a JNI global reference to ensure the pointer remains valid. Fixes #161
rib
force-pushed
the
rib/pr/sound-application-asset-manager
branch
from
d562cc5 to
44bdb8b
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
1 participant
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.
This makes sure that the
AssetManagerwe return fromAndroidApp::asset_managercan be retained with a static lifetime and never become a wrapper for an invalid pointer.The key change here is that we now return the
ApplicationAssetManager(i.e. fromApplication.getAssets()) instead of theActivityAssetManager.Theoretically there could be some applications that could associate an
ActivityAssetManagerwith unique resources but that's not expected to be common (and at least not expected to affect anyone currently usingAndroidApp::asset_manager).As part of the
APP_ONCEinitialization ininit_android_main_threadwe now get a global reference to theApplicationAssetManagerand get the correspondingAAssetManagerthat we can trust will be valid for the lifetime of the process since we leak the global reference.Note: The
ApplicationAssetManageris logically a process-wide resource and so the leaked global is just a technical formality to ensure it can't be garbage collected, but that's assumed to be redundant.Note: If anyone strictly needs the
ActivityAssetManagerthen they could at least resort to callingActivity.getAssets()via JNI manually, but perhaps we can later consider adding a separateAndroidApp::activity_asset_manager()that will pair anAAssetManagerpointer with a JNI global reference to ensure the pointer remains valid.Fixes #161