fixed #17594: [android] fix crash and memory leaks if launching game …

…multiple times on Android platform with PhysX module enabled. (#17602)
cocos · Sep 4, 2024 · 55c81c3 · 55c81c3
1 parent e2396b8
commit 55c81c3
Show file tree

Hide file tree

Showing 17 changed files with 106 additions and 10 deletions.
diff --git a/native/cocos/application/BaseGame.cpp b/native/cocos/application/BaseGame.cpp
@@ -34,6 +34,13 @@
 extern "C" void cc_load_all_plugins(); // NOLINT
 
 namespace cc {
+
+BaseGame::~BaseGame() { // NOLINT
+#if (CC_PLATFORM == CC_PLATFORM_ANDROID) && CC_SUPPORT_ADPF
+    ADPFManager::getInstance().destroy();
+#endif
+}
+
 int BaseGame::init() {
     cc::pipeline::GlobalDSManager::setDescriptorSetLayout();
 

diff --git a/native/cocos/application/BaseGame.h b/native/cocos/application/BaseGame.h
@@ -44,6 +44,7 @@ class BaseGame : public CocosApplication {
     };
 
     BaseGame() = default;
+    ~BaseGame() override;
     int init() override;
 
 protected:

diff --git a/native/cocos/physics/physx/PhysXSharedBody.cpp b/native/cocos/physics/physx/PhysXSharedBody.cpp
@@ -70,6 +70,7 @@ PhysXSharedBody *PhysXSharedBody::getSharedBody(const Node *node, PhysXWorld *co
     PhysXSharedBody *newSB;
     if (iter != sharedBodesMap.end()) {
         newSB = iter->second;
+        CC_ASSERT_EQ(newSB->_mWrappedWorld, world);
     } else {
         newSB = ccnew PhysXSharedBody(const_cast<Node *>(node), world, body);
         newSB->_mFilterData.word0 = 1;
@@ -84,6 +85,16 @@ PhysXSharedBody *PhysXSharedBody::getSharedBody(const Node *node, PhysXWorld *co
     return newSB;
 }
 
+void PhysXSharedBody::clearCache() {
+    // 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);
+    for (auto &e : tmpMap) {
+        delete e.second;
+    }
+    sharedBodesMap.clear();
+}
+
 PhysXSharedBody::~PhysXSharedBody() {
     sharedBodesMap.erase(_mNode);
     if (_mStaticActor != nullptr) PX_RELEASE(_mStaticActor);

diff --git a/native/cocos/physics/physx/PhysXSharedBody.h b/native/cocos/physics/physx/PhysXSharedBody.h
@@ -42,6 +42,7 @@ class PhysXRigidBody;
 class PhysXSharedBody final {
 public:
     static PhysXSharedBody *getSharedBody(const Node *node, PhysXWorld *world, PhysXRigidBody *body);
+    static void clearCache();
     PhysXSharedBody() = delete;
     PhysXSharedBody(const PhysXSharedBody &other) = delete;
     PhysXSharedBody(PhysXSharedBody &&other) = delete;

diff --git a/native/cocos/physics/physx/PhysXWorld.cpp b/native/cocos/physics/physx/PhysXWorld.cpp
@@ -105,6 +105,7 @@ PhysXWorld::~PhysXWorld() {
     // clear material cache
     materialMap.clear();
     delete _mEventMgr;
+    PhysXSharedBody::clearCache();
     PhysXJoint::releaseTempRigidActor();
     PX_RELEASE(_mControllerManager);
     PX_RELEASE(_mScene);
@@ -119,6 +120,7 @@ PhysXWorld::~PhysXWorld() {
     PX_RELEASE(_mCooking);
     PxCloseExtensions();
     PX_RELEASE(_mFoundation);
+    instance = nullptr;
 }
 
 void PhysXWorld::step(float fixedTimeStep) {

diff --git a/native/cocos/physics/physx/PhysXWorld.h b/native/cocos/physics/physx/PhysXWorld.h
@@ -156,7 +156,7 @@ class PhysXWorld final : virtual public IPhysicsWorld {
     physx::PxDefaultCpuDispatcher *_mDispatcher;
     physx::PxScene *_mScene;
     PhysXEventManager *_mEventMgr;
-    uint32_t _mCollisionMatrix[31];
+    uint32_t _mCollisionMatrix[31] = {0};
     ccstd::vector<PhysXSharedBody *> _mSharedBodies;
     ccstd::vector<PhysXCharacterController *> _mCCTs;
 

diff --git a/native/cocos/platform/android/adpf_manager.cpp b/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_);
+    obj_power_service_ = nullptr;
+}
+
 // Initialize JNI calls for the powermanager.
 bool ADPFManager::initializePowerManager() {
     #if __ANDROID_API__ >= 31

diff --git a/native/cocos/platform/android/adpf_manager.h b/native/cocos/platform/android/adpf_manager.h
@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-#ifndef ADPF_MANAGER_H_
-#define ADPF_MANAGER_H_
+#pragma once
 
 #if CC_PLATFORM == CC_PLATFORM_ANDROID && __ANDROID_API__ >= 30
     #include <android/api-level.h>
@@ -115,6 +114,7 @@ class ADPFManager {
     AThermalManager *getThermalManager() { return thermal_manager_; }
 
     void initialize();
+	void destroy();
 
 private:
     // Update thermal headroom each sec.
@@ -185,4 +185,3 @@ class ADPFManager {
     #define CC_SUPPORT_ADPF 0 // NOLINT
 #endif                        // ADPF_MANAGER_H_
 
-#endif
diff --git a/native/cocos/platform/android/java/libs/game-sdk.jar b/native/cocos/platform/android/java/libs/game-sdk.jar
diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosActivity.java b/native/cocos/platform/android/java/src/com/cocos/lib/CocosActivity.java
@@ -137,7 +137,22 @@ protected void onDestroy() {
         CocosHelper.unregisterBatteryLevelReceiver(this);
         CocosAudioFocusManager.unregisterAudioFocusListener(this);
         CanvasRenderingContext2DImpl.destroy();
+        CocosHelper.destroy();
         GlobalObject.destroy();
+        CocosWebViewHelper.resetStaticVariables();
+        CocosSensorHandler.resetStaticVariables();
+
+        mVideoHelper.destroy();
+        mSurfaceView.setOnTouchListener(null);
+        mSurfaceView.getHolder().removeCallback(this);
+
+        mRootLayout.removeAllViews();
+        mRootLayout = null;
+
+        mSensorHandler = null;
+        mWebViewHelper = null;
+        mVideoHelper = null;
+        mSurfaceView = null;
     }
 
     @Override

diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosHelper.java b/native/cocos/platform/android/java/src/com/cocos/lib/CocosHelper.java
@@ -78,7 +78,7 @@ public class CocosHelper {
     // ===========================================================
 
     private static Vibrator sVibrateService;
-    private static BatteryReceiver sBatteryReceiver = new BatteryReceiver();
+    private static BatteryReceiver sBatteryReceiver = null;
 
     public static final int NETWORK_TYPE_NONE = 0;
     public static final int NETWORK_TYPE_LAN = 1;
@@ -98,6 +98,13 @@ public void addTask(Runnable runnable) {
                 sTaskQ.add(runnable);
             }
         }
+
+        public void clearTasks() {
+            synchronized (readMtx) {
+                sTaskQ.clear();
+            }
+        }
+
         public void runTasks(){
             Queue<Runnable> tmp;
             synchronized (readMtx) {
@@ -135,12 +142,18 @@ public void setBatteryLevelByIntent(Intent intent) {
     }
 
     static void registerBatteryLevelReceiver(Context context) {
+        if (sBatteryReceiver == null) {
+            sBatteryReceiver = new BatteryReceiver();
+        }
         Intent intent = context.registerReceiver(sBatteryReceiver, new IntentFilter(Intent.ACTION_BATTERY_CHANGED));
         sBatteryReceiver.setBatteryLevelByIntent(intent);
     }
 
     static void unregisterBatteryLevelReceiver(Context context) {
-        context.unregisterReceiver(sBatteryReceiver);
+        if (sBatteryReceiver != null) {
+            context.unregisterReceiver(sBatteryReceiver);
+            sBatteryReceiver = null;
+        }
     }
 
     //Run on game thread forever, no matter foreground or background
@@ -197,6 +210,13 @@ public static void init() {
         }
     }
 
+    public static void destroy() {
+        sVibrateService = null;
+        sInited = false;
+        sTaskQOnGameThread.clearTasks();
+        sForegroundTaskQOnGameThread.clearTasks();
+    }
+
     public static float getBatteryLevel() {
         return sBatteryReceiver.sBatteryLevel;
     }

diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosLocalStorage.java b/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;
         if (mDatabase != null) {
             mDatabase.close();
+            mDatabase = null;
         }
     }
 

diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosSensorHandler.java b/native/cocos/platform/android/java/src/com/cocos/lib/CocosSensorHandler.java
@@ -58,6 +58,11 @@ public CocosSensorHandler(final Context context) {
         mSensorHandler = this;
     }
 
+    public static void resetStaticVariables() {
+        mSensorHandler = null;
+        mEnableSensor = false;
+    }
+
     // ===========================================================
     // Getter & Setter
     // ===========================================================

diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosVideoHelper.java b/native/cocos/platform/android/java/src/com/cocos/lib/CocosVideoHelper.java
@@ -48,7 +48,6 @@ public class CocosVideoHelper {
     private Activity mActivity = null;
     private static SparseArray<CocosVideoView> sVideoViews = null;
     static VideoHandler mVideoHandler = null;
-    private static Handler sHandler = null;
 
     CocosVideoHelper(Activity activity, FrameLayout layout)
     {
@@ -57,7 +56,16 @@ public class CocosVideoHelper {
 
         mVideoHandler = new VideoHandler(this);
         sVideoViews = new SparseArray<CocosVideoView>();
-        sHandler = new Handler(Looper.myLooper());
+    }
+
+	public void destroy() {
+        if (mVideoHandler != null) {
+            mVideoHandler.removeCallbacksAndMessages(null);
+            mVideoHandler = null;
+        }
+        videoEventListener = null;
+        mLayout = null;
+        mActivity = null;
     }
 
     private static int videoTag = 0;

diff --git a/native/cocos/platform/android/java/src/com/cocos/lib/CocosWebViewHelper.java b/native/cocos/platform/android/java/src/com/cocos/lib/CocosWebViewHelper.java
@@ -56,6 +56,15 @@ public CocosWebViewHelper(FrameLayout layout) {
         CocosWebViewHelper.webViews = new SparseArray<CocosWebView>();
     }
 
+    public static void resetStaticVariables() {
+        sLayout = null;
+        if (sHandler != null) {
+            sHandler.removeCallbacksAndMessages(null);
+            sHandler = null;
+        }
+        webViews = null;
+    }
+
     private static native boolean shouldStartLoading(int index, String message);
     private static native void didFinishLoading(int index, String message);
     private static native void didFailLoading(int index, String message);

diff --git a/native/cocos/scene/RenderWindow.cpp b/native/cocos/scene/RenderWindow.cpp
@@ -56,7 +56,17 @@ RenderWindow::RenderWindow()
   _colorName("Color" + std::to_string(_renderWindowId)),
   _depthStencilName("DepthStencil" + std::to_string(_renderWindowId)) {}
 
-RenderWindow::~RenderWindow() = default;
+RenderWindow::~RenderWindow() {
+    // NOTE: destroy needs to be invoked in the destructor of RenderWindow to avoid wild pointer issues in gfx backend code.
+    // RenderWindow owns `_frameBuffer` and `_colorTextures`, `_frameBuffer` should be released before `_colorTextures`
+    // since gfx::Framebuffer keeps a weak pointer of `_colorTextures` and there is code :
+    // GLES3Device::getInstance()->framebufferHub()->disengage(colorTexture->gpuTexture(), _gpuFBO);
+    // in the GLES3Framebuffer::doDestroy.
+    // Invoking `destroy` here will make sure that `_frameBuffer` is destructed before `_colorTextures`,
+    // otherwise, the `colorTexture` in `disengage(colorTexture->gpuTexture(), _gpuFBO);` will be a wild pointer and
+    // colorTexture->gpuTexture() will be an invalid memory read operation.
+    destroy();
+}
 
 bool RenderWindow::initialize(gfx::Device *device, IRenderWindowInfo &info) {
     if (info.title.has_value() && !info.title.value().empty()) {

diff --git a/native/external-config.json b/native/external-config.json
@@ -3,6 +3,6 @@
         "type": "github",
         "owner": "cocos-creator",
         "name": "engine-native-external",
-        "checkout": "v3.8.4-5"
+        "checkout": "v3.8.5-1"
     }
 }