fix some potential memleaks and null pointer dereferences

cocos/editor-support/spine-creator-support/SkeletonDataMgr.cpp

@@ -33,39 +33,37 @@
 
 using namespace spine;
 
-class SkeletonDataInfo;
-static std::map<std::string, SkeletonDataInfo*> _dataMap;
+namespace spine {
 
 class SkeletonDataInfo : public cocos2d::Ref {
 public:
-    SkeletonDataInfo (const std::string& uuid) {
-        _uuid = uuid;
-    }
-
-    ~SkeletonDataInfo () {
+    SkeletonDataInfo() = default;
+
+    ~SkeletonDataInfo() {
         if (data) {
             delete data;
             data = nullptr;
         }
-        
+
         if (atlas) {
             delete atlas;
             atlas = nullptr;
         }
-        
+
         if (attachmentLoader) {
             delete attachmentLoader;
             attachmentLoader = nullptr;
         }
     }
-    
-    SkeletonData* data = nullptr;
-    Atlas* atlas = nullptr;
-    AttachmentLoader* attachmentLoader = nullptr;
+
+    SkeletonData *data = nullptr;
+    Atlas *atlas = nullptr;
+    AttachmentLoader *attachmentLoader = nullptr;
     std::vector<int> texturesIndex;
-    std::string _uuid;
 };
 
+} // namespace spine
+
 SkeletonDataMgr* SkeletonDataMgr::_instance = nullptr;
 
 bool SkeletonDataMgr::hasSkeletonData (const std::string& uuid) {
@@ -78,7 +76,7 @@ void SkeletonDataMgr::setSkeletonData (const std::string& uuid, SkeletonData* da
     if (it != _dataMap.end()) {
         releaseByUUID(uuid);
     }
-    SkeletonDataInfo* info = new SkeletonDataInfo(uuid);
+    SkeletonDataInfo* info = new SkeletonDataInfo();
     info->data = data;
     info->atlas = atlas;
     info->attachmentLoader = attachmentLoader;

cocos/editor-support/spine-creator-support/SkeletonDataMgr.h

@@ -39,6 +39,8 @@
 
 namespace spine {
 
+class SkeletonDataInfo;
+
 /**
  * Cache skeleton data.
  */
@@ -73,6 +75,7 @@ class SkeletonDataMgr {
 private:
     static SkeletonDataMgr* _instance;
     destroyCallback _destroyCallback = nullptr;
+    std::map<std::string, SkeletonDataInfo*> _dataMap;
 };
 
 }

cocos/platform/android/jni/JniHelper.h

@@ -340,27 +340,28 @@ class CC_DLL JniHelper
         return "Z";
     }
 
+    // jchar is unsigned 16 bits, we do char => jchar conversion on purpose
     static std::string getJNISignature(char) {
         return "C";
     }
 
-    static std::string getJNISignature(short) {
+    static std::string getJNISignature(jshort) {
         return "S";
     }
 
-    static std::string getJNISignature(int) {
+    static std::string getJNISignature(jint) {
         return "I";
     }
 
-    static std::string getJNISignature(long) {
+    static std::string getJNISignature(jlong) {
         return "J";
     }
 
-    static std::string getJNISignature(float) {
+    static std::string getJNISignature(jfloat) {
         return "F";
     }
 
-    static std::string getJNISignature(double) {
+    static std::string getJNISignature(jdouble) {
         return "D";
     }
 

cocos/renderer/scene/StencilManager.cpp

@@ -139,7 +139,7 @@ void StencilManager::enableMask ()
 void StencilManager::exitMask ()
 {
     if (_maskStack.size() == 0) {
-        cocos2d::log("StencilManager:exitMask _maskStack:%lu size is 0", _maskStack.size());
+        cocos2d::log("StencilManager:exitMask _maskStack:%zu size is 0", _maskStack.size());
     }
     _maskStack.pop_back();
     if (_maskStack.size() == 0) {

cocos/renderer/scene/assembler/CustomAssembler.cpp

@@ -91,7 +91,7 @@ InputAssembler* CustomAssembler::adjustIA(std::size_t index)
     }
     else
     {
-        cocos2d::log("CustomAssembler:updateIA index:%lu is out of range", index);
+        cocos2d::log("CustomAssembler:updateIA index:%zu is out of range", index);
         return nullptr;
     }
 
@@ -127,7 +127,7 @@ void CustomAssembler::updateEffect(std::size_t index, EffectVariant* effect)
         _effects.replace(index, effect);
         return;
     }
-    cocos2d::log("CustomAssembler:updateEffect index:%lu out of range", index);
+    cocos2d::log("CustomAssembler:updateEffect index:%zu out of range", index);
 }
 
 RENDERER_END

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

@@ -44,11 +44,11 @@ using namespace cocos2d;
 se::Object* __jsbObj = nullptr;
 se::Object* __glObj = nullptr;
 
-static ThreadPool* __threadPool = nullptr;
+static std::shared_ptr<ThreadPool> g_threadPool;
 
-static std::shared_ptr<cocos2d::network::Downloader> _localDownloader = nullptr;
-static std::map<std::string, std::function<void(const std::string&, unsigned char*, int ,const std::string&)>> _localDownloaderHandlers;
-static uint64_t _localDownloaderTaskId = 1000000;
+static std::shared_ptr<cocos2d::network::Downloader> g_localDownloader = nullptr;
+static std::map<std::string, std::function<void(const std::string&, unsigned char*, int ,const std::string&)>> g_localDownloaderHandlers;
+static uint64_t g_localDownloaderTaskId = 1000000;
 static std::string xxteaKey = "";
 void jsb_set_xxtea_key(const std::string& key)
 {
@@ -57,19 +57,19 @@ void jsb_set_xxtea_key(const std::string& key)
 
 static cocos2d::network::Downloader *localDownloader()
 {
-    if(!_localDownloader)
+    if(!g_localDownloader)
     {
-        _localDownloader = std::make_shared<cocos2d::network::Downloader>();
-        _localDownloader->onDataTaskSuccess = [=](const cocos2d::network::DownloadTask& task,
+        g_localDownloader = std::make_shared<cocos2d::network::Downloader>();
+        g_localDownloader->onDataTaskSuccess = [=](const cocos2d::network::DownloadTask& task,
                                             std::vector<unsigned char>& data) {
             if(data.empty())
             {
                 SE_REPORT_ERROR("Getting image from (%s) failed!", task.requestURL.c_str());
                 return;
             }
 
-            auto callback = _localDownloaderHandlers.find(task.identifier);
-            if(callback == _localDownloaderHandlers.end())
+            auto callback = g_localDownloaderHandlers.find(task.identifier);
+            if(callback == g_localDownloaderHandlers.end())
             {
                 SE_REPORT_ERROR("Getting image from (%s), callback not found!!", task.requestURL.c_str());
                 return;
@@ -80,35 +80,35 @@ static cocos2d::network::Downloader *localDownloader()
 
             (callback->second)("", imageData, imageBytes, "");
             //initImageFunc("", imageData, imageBytes);
-            _localDownloaderHandlers.erase(callback);
+            g_localDownloaderHandlers.erase(callback);
         };
-        _localDownloader->onTaskError = [=](const cocos2d::network::DownloadTask& task,
+        g_localDownloader->onTaskError = [=](const cocos2d::network::DownloadTask& task,
                                       int errorCode,
                                       int errorCodeInternal,
                                       const std::string& errorStr) {
 
             SE_REPORT_ERROR("Getting image from (%s) failed!", task.requestURL.c_str());
-            auto callback = _localDownloaderHandlers.find(task.identifier);
-            if(callback == _localDownloaderHandlers.end())
+            auto callback = g_localDownloaderHandlers.find(task.identifier);
+            if(callback == g_localDownloaderHandlers.end())
             {
                 SE_REPORT_ERROR("Getting image from (%s), callback not found!!", task.requestURL.c_str());
                 return;
             }
 
             (callback->second)("", nullptr, 0, errorStr);
-            _localDownloaderHandlers.erase(task.identifier);
+            g_localDownloaderHandlers.erase(task.identifier);
         };
     }
-    return _localDownloader.get();
+    return g_localDownloader.get();
 }
 
 static void localDownloaderCreateTask(const std::string &url, std::function<void(const std::string&, unsigned char*, int, const std::string&)> callback)
 {
     std::stringstream ss;
-    ss << "jsb_loadimage_" << (_localDownloaderTaskId++);
+    ss << "jsb_loadimage_" << (g_localDownloaderTaskId++);
     std::string key = ss.str();
     auto task = localDownloader()->createDownloadDataTask(url, key);
-    _localDownloaderHandlers.emplace(std::make_pair(task->identifier, callback));
+    g_localDownloaderHandlers.emplace(std::make_pair(task->identifier, callback));
 }
 
 static const char* BYTE_CODE_FILE_EXT = ".jsc";
@@ -831,36 +831,42 @@ bool jsb_global_load_image(const std::string& path, const se::Value& callbackVal
     std::shared_ptr<se::Value> callbackPtr = std::make_shared<se::Value>(callbackVal);
 
     auto initImageFunc = [path, callbackPtr](const std::string& fullPath, unsigned char* imageData, int imageBytes, const std::string& errorMsg){
-        Image* img = new (std::nothrow) Image();
+        std::shared_ptr<uint8_t> imageDataGuard(imageData, free);
 
-        __threadPool->pushTask([=](int tid){
+        auto pool = g_threadPool;
+        if (!pool)
+            return;
+        pool->pushTask([=](int tid) mutable {
             // NOTE: FileUtils::getInstance()->fullPathForFilename isn't a threadsafe method,
             // Image::initWithImageFile will call fullPathForFilename internally which may
             // cause thread race issues. Therefore, we get the full path of file before
             // going into task callback.
             // Be careful of invoking any Cocos2d-x interface in a sub-thread.
             bool loadSucceed = false;
+            std::shared_ptr<Image> img(new Image(), [](Image *image) {
+                image->release();
+            });
 
             if (!errorMsg.empty()) {
                 loadSucceed = false;
             }
             else if (fullPath.empty())
             {
-                loadSucceed = img->initWithImageData(imageData, imageBytes);
-                free(imageData);
+                loadSucceed = img->initWithImageData(imageDataGuard.get(), imageBytes);
+                imageDataGuard = nullptr;
             }
             else
             {
                 loadSucceed = img->initWithImageFile(fullPath);
             }
 
-            struct ImageInfo* imgInfo = nullptr;
+            std::shared_ptr<ImageInfo> imgInfo;
             if(loadSucceed)
             {
-                imgInfo = createImageInfo(img);
+                imgInfo.reset(createImageInfo(img.get()));
             }
 
-            Application::getInstance()->getScheduler()->performFunctionInCocosThread([=](){
+            Application::getInstance()->getScheduler()->performFunctionInCocosThread([=]() mutable {
                 se::AutoHandleScope hs;
                 se::ValueArray seArgs;
                 se::Value dataVal;
@@ -900,7 +906,7 @@ bool jsb_global_load_image(const std::string& path, const se::Value& callbackVal
 
                     seArgs.push_back(se::Value(retObj));
 
-                    delete imgInfo;
+                    imgInfo = nullptr;
                 }
                 else
                 {
@@ -914,7 +920,7 @@ bool jsb_global_load_image(const std::string& path, const se::Value& callbackVal
                 }
 
                 callbackPtr->toObject()->call(seArgs, nullptr);
-                img->release();
+                img = nullptr;
             });
 
         });
@@ -1232,7 +1238,7 @@ SE_BIND_FUNC(JSB_hideInputBox)
 
 bool jsb_register_global_variables(se::Object* global)
 {
-    __threadPool = ThreadPool::newFixedThreadPool(3);
+    g_threadPool.reset(ThreadPool::newFixedThreadPool(3));
 
     global->defineFunction("require", _SE(require));
     global->defineFunction("requireModule", _SE(moduleRequire));
@@ -1280,8 +1286,7 @@ bool jsb_register_global_variables(se::Object* global)
     se::ScriptEngine::getInstance()->clearException();
 
     se::ScriptEngine::getInstance()->addBeforeCleanupHook([](){
-        delete __threadPool;
-        __threadPool = nullptr;
+        g_threadPool = nullptr;
 
         PoolManager::getInstance()->getCurrentPool()->clear();
     });