Fixing serious XMLHTTP leak when CC_ENABLE_GC_FOR_NATIVE_OBJECTS enabled (#17844)

* Fixing issue where XMLHttpRequests would not have their destructor fire if CC_ENABLE_GC_FOR_NATIVE_OBJECTS was set. This was due to mismanagement of JSHeap object tracing.

* Removing unused variables in js_add_object_root.

Author: DavidDeSimone

cocos/scripting/js-bindings/manual/cocos2d_specifics.cpp

@@ -667,7 +667,7 @@ void js_add_object_root(JS::HandleValue target)
     }
 
     JS::RootedObject root(cx);
-    get_or_create_js_obj(cx, jsbObj, "jsb._root", &root);
+    get_or_create_js_obj(cx, jsbObj, "_root", &root);
     JS::RootedValue valRoot(cx, OBJECT_TO_JSVAL(root));
 
     JS::RootedValue retval(cx);
@@ -688,12 +688,6 @@ void js_remove_object_root(JS::HandleValue target)
     JSContext *cx = engine->getGlobalContext();
     JS::RootedObject global(cx, engine->getGlobalObject());
     JSAutoCompartment(cx, global);
-    JS::RootedObject targetObj(cx, target.toObjectOrNull());
-    js_proxy_t *pTarget = jsb_get_js_proxy(targetObj);
-    if (!pTarget)
-    {
-        return;
-    }
 
     JS::RootedObject jsbObj(cx);
     get_or_create_js_obj(cx, global, "jsb", &jsbObj);

cocos/scripting/js-bindings/manual/network/XMLHTTPRequest.cpp

@@ -248,6 +248,7 @@ void MinXmlHttpRequest::handle_requestResponse(cocos2d::network::HttpClient *sen
                 callback.set(_onloadendCallback);
                 _notify(callback, JS::HandleValueArray::empty());
             }
+            _clearCallbacks();
             return;
         }
     }
@@ -291,6 +292,7 @@ void MinXmlHttpRequest::handle_requestResponse(cocos2d::network::HttpClient *sen
         callback.set(_onloadendCallback);
         _notify(callback, JS::HandleValueArray::empty());
     }
+    _clearCallbacks();
 }
 /**
  * @brief   Send out request and fire callback when done.
@@ -303,6 +305,27 @@ void MinXmlHttpRequest::_sendRequest(JSContext *cx)
     _httpRequest->release();
 }
 
+#define REMOVE_CALLBACK(x) \
+if (x)\
+{ \
+    JS::RootedValue callback(_cx); \
+    callback.set(OBJECT_TO_JSVAL(x)); \
+    js_remove_object_root(callback); \
+    x = nullptr; \
+} \
+
+void MinXmlHttpRequest::_clearCallbacks()
+{ 
+    JSB_AUTOCOMPARTMENT_WITH_GLOBAL_OBJCET
+    REMOVE_CALLBACK(_onreadystateCallback)
+    REMOVE_CALLBACK(_onloadstartCallback)
+    REMOVE_CALLBACK(_onabortCallback)
+    REMOVE_CALLBACK(_onerrorCallback)
+    REMOVE_CALLBACK(_onloadCallback)
+    REMOVE_CALLBACK(_onloadendCallback)
+    REMOVE_CALLBACK(_ontimeoutCallback)
+}
+
 MinXmlHttpRequest::MinXmlHttpRequest()
 : _url()
 {
@@ -352,49 +375,7 @@ MinXmlHttpRequest::MinXmlHttpRequest(JSContext *cx)
  */
 MinXmlHttpRequest::~MinXmlHttpRequest()
 {
-    JS::RootedValue callback(_cx);
-    if (_onreadystateCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onreadystateCallback));
-        js_remove_object_root(callback);
-    }
-    if (_onloadstartCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onloadstartCallback));
-        js_remove_object_root(callback);
-    }
-    if (_onabortCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onabortCallback));
-        js_remove_object_root(callback);
-    }
-    if (_onerrorCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onerrorCallback));
-        js_remove_object_root(callback);
-    }
-    if (_onloadCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onloadCallback));
-        js_remove_object_root(callback);
-    }
-    if (_onloadendCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_onloadendCallback));
-        js_remove_object_root(callback);
-    }
-    if (_ontimeoutCallback)
-    {
-        callback.set(OBJECT_TO_JSVAL(_ontimeoutCallback));
-        js_remove_object_root(callback);
-    }
-
-    if (_httpRequest)
-    {
-        // We don't need to release _httpRequest here since it will be released in the http callback.
-//        _httpRequest->release();
-    }
-
+    _clearCallbacks();
     CC_SAFE_FREE(_data);
     CC_SAFE_RELEASE_NULL(_scheduler);
 }
@@ -890,6 +871,7 @@ void MinXmlHttpRequest::update(float dt)
         {
             JS::RootedObject callback(_cx, _ontimeoutCallback);
             _notify(callback, JS::HandleValueArray::empty());
+            _clearCallbacks();
         }
         _elapsedTime = 0;
         _readyState = UNSENT;
@@ -916,6 +898,7 @@ JS_BINDED_FUNC_IMPL(MinXmlHttpRequest, abort)
     {
         JS::RootedObject callback(cx, _onabortCallback);
         _notify(callback, JS::HandleValueArray::empty());
+        _clearCallbacks();
     }
 
     return true;

cocos/scripting/js-bindings/manual/network/XMLHTTPRequest.h

@@ -96,6 +96,7 @@ class MinXmlHttpRequest : public cocos2d::Ref
     void _setHttpRequestData(const char *data, size_t len);
     void _sendRequest(JSContext *cx);
     void _notify(JS::HandleObject callback, JS::HandleValueArray args);
+    void _clearCallbacks();
 
     std::string                       _url;
     JSContext*                        _cx;