fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. #17602

dumganhar · 2024-09-04T07:11:19Z

Re: #17594

cocos/google-game-sdk#9
cocos/cocos-engine-external#458

Changelog

Continuous Integration

This pull request:

needs automatic test cases check.

Manual trigger with @cocos-robot run test cases afterward.
does not change any runtime related code or build configuration

If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.

Compatibility Check

This pull request:

changes public API, and have ensured backward compatibility with deprecated features.
affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
affects file structure of the build package or build configuration which requires user project upgrade.
introduces breaking changes, please list all changes, affected features and the scope of violation.

Greptile Summary

This pull request updates the external dependency version for the engine-native-external repository, addressing crash and memory leak issues on Android platforms with PhysX module enabled.

Updated native/external-config.json to use engine-native-external version v3.8.5-1, up from v3.8.4-5
This change likely incorporates fixes for the ADPFManager class, including proper resource cleanup and improved header inclusion

github-actions · 2024-09-04T07:19:55Z

Interface Check Report

This pull request does not change any public interfaces !

greptile-apps

_{16 file(s) reviewed, 6 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-09-04T07:15:22Z

native/cocos/physics/physx/PhysXSharedBody.cpp

+    // Move the map to avoid erase operation in the following for loop since 'delete' will trigger ~PhysxSharedBody.
+    // clearCache is invoked only in the destructor of PhysXWorld.
+    auto tmpMap = std::move(sharedBodesMap);


style: Consider using a unique_ptr for automatic memory management of PhysXSharedBody instances

greptile-apps · 2024-09-04T07:19:02Z

native/cocos/platform/android/adpf_manager.cpp

@@ -87,6 +87,12 @@ void ADPFManager::initialize() {
    }
 }

+void ADPFManager::destroy() {
+    JNIEnv *env = cc::JniHelper::getEnv();
+    env->DeleteGlobalRef(obj_power_service_);


style: Consider checking if obj_power_service_ is not null before deleting

greptile-apps · 2024-09-04T07:22:23Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosLocalStorage.java

    public static void destroy() {
+        mDatabaseOpenHelper = null;
        if (mDatabase != null) {
            mDatabase.close();
+            mDatabase = null;
        }


style: Consider adding a null check before closing mDatabase to prevent potential NullPointerException

greptile-apps · 2024-09-04T07:23:28Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosSensorHandler.java

+    public static void resetStaticVariables() {
+        mSensorHandler = null;
+        mEnableSensor = false;
+    }


style: Consider adding synchronization to prevent potential race conditions when resetting static variables.

greptile-apps · 2024-09-04T07:24:40Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosVideoHelper.java

+        if (mVideoHandler != null) {
+            mVideoHandler.removeCallbacksAndMessages(null);
+            mVideoHandler = null;
+        }


style: Consider using a more specific method like removeCallbacks() instead of removeCallbacksAndMessages(null) if possible

greptile-apps · 2024-09-04T07:26:13Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosWebViewHelper.java

+            sHandler.removeCallbacksAndMessages(null);
+            sHandler = null;


style: Ensure all callbacks are removed before setting sHandler to null to prevent potential memory leaks

greptile-apps

LGTM

_{2 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

LGTM

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

cocos/google-game-sdk#9

native/cocos/scene/RenderWindow.cpp

greptile-apps

_{18 file(s) reviewed, 4 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-09-04T09:06:42Z

native/cocos/physics/physx/PhysXSharedBody.cpp

+    // Move the map to avoid erase operation in the following for loop since 'delete' will trigger ~PhysxSharedBody.
+    // clearCache is invoked only in the destructor of PhysXWorld.
+    auto tmpMap = std::move(sharedBodesMap);


style: Consider using a unique_ptr for automatic memory management of PhysXSharedBody instances

greptile-apps · 2024-09-04T09:23:31Z

native/cocos/platform/android/adpf_manager.cpp

@@ -87,6 +87,12 @@ void ADPFManager::initialize() {
    }
 }

+void ADPFManager::destroy() {
+    JNIEnv *env = cc::JniHelper::getEnv();
+    env->DeleteGlobalRef(obj_power_service_);


style: Consider checking if obj_power_service_ is not null before deleting

greptile-apps · 2024-09-04T09:43:39Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosLocalStorage.java

@@ -53,8 +53,10 @@ public static boolean init(String dbName, String tableName) {
    }

    public static void destroy() {
+        mDatabaseOpenHelper = null;


style: Consider adding a null check before setting mDatabaseOpenHelper to null

greptile-apps · 2024-09-04T09:56:42Z

native/cocos/platform/android/java/src/com/cocos/lib/CocosVideoHelper.java

+        if (mVideoHandler != null) {
+            mVideoHandler.removeCallbacksAndMessages(null);
+            mVideoHandler = null;
+        }


style: Consider using a more specific method like removeCallbacks() instead of removeCallbacksAndMessages(null) if possible

greptile-apps

LGTM

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

dumganhar · 2024-09-05T06:37:29Z

@cocos-robot run test cases

github-actions · 2024-09-05T08:04:26Z

@dumganhar, Please check the result of run test cases:

Test Platform: PR-Test
Editor Version: v3.8.5
Task Result: FAIL
Task URL: https://cctest.cocos.org/#/reportview/PR-TEST/63dfb1a1-8ff1-4eee-83b3-cbbefa2199e9/-1
GitHub Action: https://github.com/cocos/cocos-engine/actions/runs/10715192754

Task Details

Platform	build	boot	runned	FailScene
web-mobile	PASS	PASS	FAIL	Hexa,puzzle,shield-node
ios	PASS	PASS	FAIL	Hexa,puzzle,shield-node
mac	PASS	PASS	FAIL	Hexa,puzzle,shield-node

github-actions · 2024-09-05T08:04:27Z

@dumganhar, Please check the result of run test cases:

Test Platform: PR-Test
Editor Version: v3.8.5
Task Result: FAIL
Task URL: https://cctest.cocos.org/#/reportview/PR-TEST/5a52544c-3529-4684-9581-d1b014b9df93/-1
GitHub Action: https://github.com/cocos/cocos-engine/actions/runs/10715192754

Task Details

Platform	build	boot	runned	FailScene
windows	PASS	PASS	FAIL	Hexa,puzzle,shield-node
android	PASS	PASS	FAIL	Hexa,puzzle,shield-node
wechatgame	PASS	FAIL	FAIL

dumganhar requested a review from minggo September 4, 2024 07:11

dumganhar changed the base branch from v3.8.4 to v3.8.5 September 4, 2024 07:11

greptile-apps bot reviewed Sep 4, 2024

View reviewed changes

dumganhar changed the title ~~fixed #17594: fix crash if launching game multiple times on Android platform with PhysX module enabled.~~ fixed #17594: fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. Sep 4, 2024

dumganhar changed the title ~~fixed #17594: fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled.~~ fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. Sep 4, 2024

dumganhar added 8 commits September 4, 2024 17:00

Release ADPF in ~BaseGame.

6f099c9

Add PhysXSharedBody::clearCache, invoke it when PhysXWorld is released.

ba62d1d

Fix the release order of variables in RenderWindow

ee78f83

Update game-sdk jar, add GameControllerManager.terminate

08ca142

cocos/google-game-sdk#9

Add ADPFManager::destroy

96f85e5

Fix memory leak for android platform.

f832734

Fix clang-tidy

4820a00

Use #pragma once.

04ca338

dumganhar force-pushed the 385-update-game-sdk-jar branch from 2622a92 to 04ca338 Compare September 4, 2024 09:00

minggo reviewed Sep 4, 2024

View reviewed changes

native/cocos/scene/RenderWindow.cpp Show resolved Hide resolved

minggo approved these changes Sep 4, 2024

View reviewed changes

Update external-config.json

8271229

greptile-apps bot reviewed Sep 4, 2024

View reviewed changes

dumganhar merged commit 55c81c3 into cocos:v3.8.5 Sep 4, 2024
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. #17602

fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. #17602

dumganhar commented Sep 4, 2024 •

edited by greptile-apps bot

Loading

github-actions bot commented Sep 4, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot Sep 4, 2024

greptile-apps bot left a comment

dumganhar commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. #17602

fixed #17594: [android] fix crash and memory leaks if launching game multiple times on Android platform with PhysX module enabled. #17602

Conversation

dumganhar commented Sep 4, 2024 • edited by greptile-apps bot Loading

Changelog

Continuous Integration

Compatibility Check

Greptile Summary

github-actions bot commented Sep 4, 2024 • edited Loading

Interface Check Report

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 4, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

dumganhar commented Sep 5, 2024

github-actions bot commented Sep 5, 2024

Task Details

github-actions bot commented Sep 5, 2024

Task Details

dumganhar commented Sep 4, 2024 •

edited by greptile-apps bot

Loading

github-actions bot commented Sep 4, 2024 •

edited

Loading