Fix bug accessing externalHandle from multithreading. (#344)

(cherry picked from commit 9296b81)

include/pag/pag.h

@@ -244,6 +244,8 @@ class Transform;
 
 class PAGFile;
 
+struct ExternalHandle;
+
 class PAG_API PAGLayer : public Content {
  public:
   PAGLayer(std::shared_ptr<File> file, Layer* layer);
@@ -416,7 +418,7 @@ class PAG_API PAGLayer : public Content {
   virtual bool isPAGFile() const;
 
   // internal use only.
-  void* externalHandle = nullptr;
+  ExternalHandle* externalHandle = nullptr;
   std::shared_ptr<File> getFile() const;
 
  protected:

src/base/utils/ExternalHandle.h

@@ -0,0 +1,28 @@
+/////////////////////////////////////////////////////////////////////////////////////////////////
+//
+//  Tencent is pleased to support the open source community by making libpag available.
+//
+//  Copyright (C) 2021 THL A29 Limited, a Tencent company. All rights reserved.
+//
+//  Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
+//  except in compliance with the License. You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  unless required by applicable law or agreed to in writing, software distributed under the
+//  license is distributed on an "as is" basis, without warranties or conditions of any kind,
+//  either express or implied. see the license for the specific language governing permissions
+//  and limitations under the license.
+//
+/////////////////////////////////////////////////////////////////////////////////////////////////
+
+#pragma once
+
+#include <mutex>
+
+namespace pag {
+struct ExternalHandle {
+  std::mutex locker;
+  void* nativeHandle = nullptr;
+};
+}  // namespace pag

src/platform/android/JNIHelper.cpp

@@ -23,6 +23,7 @@
 #include <cassert>
 #include <string>
 #include "JPAGLayerHandle.h"
+#include "base/utils/ExternalHandle.h"
 
 static constexpr int BITMAP_FLAGS_ALPHA_UNPREMUL = 2;
 static constexpr int BITMAP_FLAGS_IS_HARDWARE = 1 << 31;
@@ -130,9 +131,10 @@ jobject ToPAGLayerJavaObject(JNIEnv* env, std::shared_ptr<pag::PAGLayer> pagLaye
   if (env == nullptr || pagLayer == nullptr) {
     return nullptr;
   }
-  if (pagLayer->externalHandle != nullptr &&
-      !env->IsSameObject(static_cast<jobject>(pagLayer->externalHandle), nullptr)) {
-    return static_cast<jobject>(pagLayer->externalHandle);
+  std::lock_guard<std::mutex> autoLocker(pagLayer->externalHandle->locker);
+  auto obj = static_cast<jobject>(pagLayer->externalHandle->nativeHandle);
+  if (obj && !env->IsSameObject(obj, nullptr)) {
+    return obj;
   }
   jobject layerObject = nullptr;
   switch (pagLayer->layerType()) {
@@ -188,8 +190,7 @@ jobject ToPAGLayerJavaObject(JNIEnv* env, std::shared_ptr<pag::PAGLayer> pagLaye
       break;
     }
   }
-  auto gObject = env->NewWeakGlobalRef(layerObject);
-  pagLayer->externalHandle = gObject;
+  pagLayer->externalHandle->nativeHandle = env->NewWeakGlobalRef(layerObject);
   return layerObject;
 }
 

src/platform/android/JPAGLayer.cpp

@@ -18,6 +18,7 @@
 
 #include "JNIHelper.h"
 #include "JPAGLayerHandle.h"
+#include "base/utils/ExternalHandle.h"
 
 static jfieldID PAGLayer_nativeContext;
 
@@ -49,8 +50,9 @@ PAG_API void Java_org_libpag_PAGLayer_nativeInit(JNIEnv* env, jclass clazz) {
 PAG_API void Java_org_libpag_PAGLayer_nativeRelease(JNIEnv* env, jobject thiz) {
   auto pagLayer = GetPAGLayer(env, thiz);
   if (pagLayer != nullptr) {
-    env->DeleteWeakGlobalRef(static_cast<jobject>(pagLayer->externalHandle));
-    pagLayer->externalHandle = nullptr;
+    std::lock_guard<std::mutex> autoLocker(pagLayer->externalHandle->locker);
+    env->DeleteWeakGlobalRef(static_cast<jobject>(pagLayer->externalHandle->nativeHandle));
+    pagLayer->externalHandle->nativeHandle = nullptr;
   }
   SetPAGLayer(env, thiz, nullptr);
 }

src/platform/cocoa/private/PAGLayerImpl.mm

@@ -24,6 +24,7 @@
 #import "PAGShapeLayerImpl.h"
 #import "PAGSolidLayerImpl.h"
 #import "PAGTextLayerImpl.h"
+#import "base/utils/ExternalHandle.h"
 #import "platform/cocoa/PAGFile.h"
 #import "platform/cocoa/PAGImageLayer.h"
 #import "platform/cocoa/PAGShapeLayer.h"
@@ -164,8 +165,9 @@ + (PAGLayer*)ToPAGLayer:(std::shared_ptr<pag::PAGLayer>)layer {
   if (layer == nullptr) {
     return nil;
   }
-  if (layer->externalHandle != nullptr) {
-    return (PAGLayer*)layer->externalHandle;
+  std::lock_guard<std::mutex> autoLocker(layer->externalHandle->locker);
+  if (layer->externalHandle->nativeHandle) {
+    return static_cast<PAGLayer*>(layer->externalHandle->nativeHandle);
   }
   id result = nil;
   switch (layer->layerType()) {
@@ -201,7 +203,7 @@ + (PAGLayer*)ToPAGLayer:(std::shared_ptr<pag::PAGLayer>)layer {
       result = [[PAGLayer alloc] initWithImpl:impl];
     } break;
   }
-  layer->externalHandle = result;
+  layer->externalHandle->nativeHandle = result;
   [result autorelease];
   return result;
 }
@@ -224,7 +226,8 @@ + (PAGLayer*)ToPAGLayer:(std::shared_ptr<pag::PAGLayer>)layer {
 }
 
 - (void)dealloc {
-  _pagLayer->externalHandle = nullptr;
+  std::lock_guard<std::mutex> autoLocker(_pagLayer->externalHandle->locker);
+  _pagLayer->externalHandle->nativeHandle = nullptr;
   [super dealloc];
 }
 

src/rendering/layers/PAGLayer.cpp

@@ -16,6 +16,7 @@
 //
 /////////////////////////////////////////////////////////////////////////////////////////////////
 
+#include "base/utils/ExternalHandle.h"
 #include "base/utils/MatrixUtil.h"
 #include "base/utils/TimeUtil.h"
 #include "base/utils/UniqueID.h"
@@ -25,11 +26,11 @@
 #include "rendering/layers/PAGStage.h"
 #include "rendering/renderers/TrackMatteRenderer.h"
 #include "rendering/utils/LockGuard.h"
-#include "rendering/utils/ScopedLock.h"
 
 namespace pag {
 PAGLayer::PAGLayer(std::shared_ptr<File> file, Layer* layer)
     : layer(layer), file(std::move(file)), _uniqueID(UniqueID::Next()) {
+  externalHandle = new ExternalHandle();
   layerMatrix.setIdentity();
   if (layer != nullptr) {  // could be nullptr.
     layerCache = LayerCache::Get(layer);
@@ -43,6 +44,7 @@ PAGLayer::~PAGLayer() {
     _trackMatteLayer->detachFromTree();
     _trackMatteLayer->trackMatteOwner = nullptr;
   }
+  delete externalHandle;
 }
 
 uint32_t PAGLayer::uniqueID() const {