fix: Improve C++ safety by attaching Cache Invalidator to jsi::Runtime's lifecycle (#1488)

* fix: fix C++ lint

* fix: attach `InvalidateCacheOnDestroy` to `jsi::Runtime`

.github/workflows/validate-cpp.yml

@@ -30,6 +30,7 @@ jobs:
             ,-readability/todo\
             ,-build/namespaces\
             ,-whitespace/comments\
+            ,-runtime/references\
             ,-build/include_order\
             ,-build/c++11\
             "

android/src/main/cpp/FrameProcessorRuntimeManager.cpp

@@ -15,6 +15,7 @@
 #include "JSIJNIConversion.h"
 #include "java-bindings/JImageProxy.h"
 #include "java-bindings/JFrameProcessorPlugin.h"
+#include "JSITypedArray.h"
 
 namespace vision {
 
@@ -151,6 +152,12 @@ void FrameProcessorRuntimeManager::installJSIBindings() {
 
   auto& jsiRuntime = *_jsRuntime;
 
+  // HostObject that attaches the cache to the lifecycle of the Runtime. On Runtime destroy, we destroy the cache.
+  auto propNameCacheObject = std::make_shared<vision::InvalidateCacheOnDestroy>(jsiRuntime);
+  jsiRuntime.global().setProperty(jsiRuntime,
+                                  "__visionCameraPropNameCache",
+                                  jsi::Object::createFromHostObject(jsiRuntime, propNameCacheObject));
+
   auto setFrameProcessor = JSI_HOST_FUNCTION_LAMBDA {
     __android_log_write(ANDROID_LOG_INFO, TAG, "Setting new Frame Processor...");
 

android/src/main/cpp/FrameProcessorRuntimeManager.h

@@ -45,7 +45,7 @@ class FrameProcessorRuntimeManager : public jni::HybridClass<FrameProcessorRunti
   void registerPlugin(alias_ref<JFrameProcessorPlugin::javaobject> plugin);
   void logErrorToJS(const std::string& message);
 
-  void setFrameProcessor(jsi::Runtime& runtime,                 // NOLINT(runtime/references)
+  void setFrameProcessor(jsi::Runtime& runtime,
                          int viewTag,
                          const jsi::Value& frameProcessor);
   void unsetFrameProcessor(int viewTag);

android/src/main/cpp/JSIJNIConversion.h

@@ -14,9 +14,9 @@ namespace JSIJNIConversion {
 
 using namespace facebook;
 
-jobject convertJSIValueToJNIObject(jsi::Runtime& runtime, const jsi::Value& value); // NOLINT(runtime/references)
+jobject convertJSIValueToJNIObject(jsi::Runtime& runtime, const jsi::Value& value);
 
-jsi::Value convertJNIObjectToJSIValue(jsi::Runtime& runtime, const jni::local_ref<jobject>& object); // NOLINT(runtime/references)
+jsi::Value convertJNIObjectToJSIValue(jsi::Runtime& runtime, const jni::local_ref<jobject>& object);
 
 } // namespace JSIJNIConversion
 

cpp/JSITypedArray.cpp

@@ -12,6 +12,11 @@
 #include "JSITypedArray.h"
 
 #include <unordered_map>
+#include <memory>
+#include <utility>
+#include <vector>
+#include <algorithm>
+#include <string>
 
 namespace vision {
 
@@ -97,7 +102,7 @@ TypedArrayKind TypedArrayBase::getKind(jsi::Runtime &runtime) const {
                              .asString(runtime)
                              .utf8(runtime);
   return getTypedArrayKindForName(constructorName);
-};
+}
 
 size_t TypedArrayBase::size(jsi::Runtime &runtime) const {
   return getProperty(runtime, propNameIDCache.get(runtime, Prop::Length)).asNumber();
@@ -191,13 +196,13 @@ void arrayBufferUpdate(
 }
 
 template <TypedArrayKind T>
-TypedArray<T>::TypedArray(jsi::Runtime &runtime, size_t size) : TypedArrayBase(runtime, size, T){};
+TypedArray<T>::TypedArray(jsi::Runtime &runtime, size_t size) : TypedArrayBase(runtime, size, T) {}
 
 template <TypedArrayKind T>
 TypedArray<T>::TypedArray(jsi::Runtime &runtime, std::vector<ContentType<T>> data)
     : TypedArrayBase(runtime, data.size(), T) {
   update(runtime, data);
-};
+}
 
 template <TypedArrayKind T>
 TypedArray<T>::TypedArray(TypedArrayBase &&base) : TypedArrayBase(std::move(base)) {}

cpp/JSITypedArray.h

@@ -13,6 +13,8 @@
 #pragma once
 
 #include <jsi/jsi.h>
+#include <utility>
+#include <vector>
 
 namespace jsi = facebook::jsi;
 
@@ -77,7 +79,7 @@ struct typedArrayTypeMap<TypedArrayKind::Float64Array> {
 // the cache object is connected to the lifecycle of the js runtime
 class InvalidateCacheOnDestroy : public jsi::HostObject {
  public:
-  InvalidateCacheOnDestroy(jsi::Runtime &runtime);
+  explicit InvalidateCacheOnDestroy(jsi::Runtime &runtime);
   virtual ~InvalidateCacheOnDestroy();
   virtual jsi::Value get(jsi::Runtime &, const jsi::PropNameID &name) {
     return jsi::Value::null();
@@ -139,9 +141,9 @@ void arrayBufferUpdate(
 template <TypedArrayKind T>
 class TypedArray : public TypedArrayBase {
  public:
+  explicit TypedArray(TypedArrayBase &&base);
   TypedArray(jsi::Runtime &runtime, size_t size);
   TypedArray(jsi::Runtime &runtime, std::vector<ContentType<T>> data);
-  TypedArray(TypedArrayBase &&base);
   TypedArray(TypedArray &&) = default;
   TypedArray &operator=(TypedArray &&) = default;
 

ios/Frame Processor/FrameProcessorRuntimeManager.mm

@@ -27,6 +27,7 @@
 #import "FrameProcessorUtils.h"
 #import "FrameProcessorCallback.h"
 #import "../React Utils/JSIUtils.h"
+#import "../../cpp/JSITypedArray.h"
 
 // Forward declarations for the Swift classes
 __attribute__((objc_runtime_name("_TtC12VisionCamera12CameraQueues")))
@@ -130,6 +131,12 @@ __attribute__((objc_runtime_name("_TtC12VisionCamera10CameraView")))
   }
 
   jsi::Runtime& jsiRuntime = *(jsi::Runtime*)cxxBridge.runtime;
+  
+  // HostObject that attaches the cache to the lifecycle of the Runtime. On Runtime destroy, we destroy the cache.
+  auto propNameCacheObject = std::make_shared<vision::InvalidateCacheOnDestroy>(jsiRuntime);
+  jsiRuntime.global().setProperty(jsiRuntime,
+                                  "__visionCameraPropNameCache",
+                                  jsi::Object::createFromHostObject(jsiRuntime, propNameCacheObject));
 
   // Install the Worklet Runtime in the main React JS Runtime
   [self setupWorkletContext:jsiRuntime];

scripts/cpplint.sh

@@ -1,7 +1,7 @@
 #!/bin/bash
 
 if which cpplint >/dev/null; then
-  cpplint --linelength=230 --filter=-legal/copyright,-readability/todo,-build/namespaces,-whitespace/comments,-build/include_order,-build/c++11 --quiet --recursive cpp android/src/main/cpp
+  cpplint --linelength=230 --filter=-legal/copyright,-readability/todo,-build/namespaces,-runtime/references,-whitespace/comments,-build/include_order,-build/c++11 --quiet --recursive cpp android/src/main/cpp
 else
   echo "warning: cpplint not installed, download from https://github.com/cpplint/cpplint"
 fi