Merge branch 'main' into nested

README.md

@@ -386,7 +386,7 @@ NOBIND_MODULE(buffer, m) {
 
 When C++ returns a `Buffer` object, that buffer is considered owned and it will be freed upon the destruction of the Node.js `Buffer` object by the garbage-collector.
 
-When JavaScript passes a `Buffer` to a C++ method, data is copied - which is the safest but not the most efficient way to transfer it. Sharing the memory between C++ and JavaScript is possible in many cases - but must be carefully implemented in a custom typemap - especially when using async.
+When JavaScript passes a `Buffer` to a C++ method, C++ receives a pointer to the underlying data region of the JS `Buffer` which is protected from the GC for duration of the call - including in async mode.
 
 ### Returning objects and factory functions
 

include/nobuffer.h

@@ -7,48 +7,31 @@ namespace Nobind {
 
 namespace Typemap {
 
-// These typemaps transfer Buffers by copying the underlying data
-// It is also possible to do it without copying if properly managing the GC
-// but this requires very through understanding of all the underlying
-// mechanisms by the user of nobind17
-
 // In C++ a Node::Buffer decomposes to std::pair<uint8_t *, size_t>
 using Buffer = std::pair<uint8_t *, size_t>;
 
-// When calling C++ with a JS Buffer, a copy of the Buffer
-// is created for the duration of the call
-// This means that the typemap is completely safe for use with async
-// code but it is not the most efficient way to transfer data
+// When calling C++ with a JS Buffer, C++ receives a pointer
+// to the Node.js Buffer region
+// The JS Buffer object is protected from the GC for the
+// duration of the call
 template <> class FromJS<Buffer> {
   Buffer val_;
+  Napi::ObjectReference persistent;
 
 public:
   inline explicit FromJS(Napi::Value val) {
     if (!val.IsBuffer()) {
       throw Napi::TypeError::New(val.Env(), "Expected a Buffer");
     }
     Napi::Buffer buf = val.As<Napi::Buffer<uint8_t>>();
-    val_ = {new uint8_t[buf.ByteLength()], buf.ByteLength()};
-    memcpy(val_.first, buf.Data() + buf.ByteOffset(), buf.ByteLength());
+    val_ = {buf.Data(), buf.ByteLength()};
+    persistent = Napi::Persistent(val.As<Napi::Object>());
   }
 
   inline Buffer Get() { return val_; }
 
-  inline ~FromJS() {
-    delete[] val_.first;
-    val_.first = nullptr;
-    val_.second = 0;
-  }
-
   FromJS(const FromJS &) = delete;
-
-  // When moving, we must be sure that the old copy
-  // won't get freed
-  inline FromJS(FromJS &&rhs) {
-    val_ = rhs.val_;
-    rhs.val_.first = nullptr;
-    rhs.val_.second = 0;
-  };
+  inline FromJS(FromJS &&) = default;
 };
 
 // When receiving a Buffer from C++ we consider that
@@ -62,11 +45,13 @@ template <const ReturnAttribute &RETATTR> class ToJS<Buffer, RETATTR> {
   inline Napi::Value Get() {
 #ifdef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
     // Node-API does not support external buffers (Electron)
+    // C++ receives a copy with the original being freed immediately
     Napi::Buffer buffer = Napi::Buffer<uint8_t>::Copy(env_, val_.first, val_.second);
     delete[] val->data;
     return buffer;
 #else
     // Node-API supports external buffers (Node.js)
+    // C++ receives ownership of the buffer which is freed upon collection of the JS Buffer object by the GC
     return Napi::Buffer<uint8_t>::New(env_, val_.first, val_.second, [](Napi::Env, uint8_t *data) { delete[] data; });
 #endif
   }

include/noobject.h

@@ -8,6 +8,8 @@
 #include <nofunction.h>
 #include <notypes.h>
 
+using namespace std::literals::string_literals;
+
 namespace Nobind {
 
 struct EmptyEnvInstanceData {};
@@ -375,7 +377,7 @@ NoObjectWrap<CLASS>::NoObjectWrap(const Napi::CallbackInfo &info) : Napi::Object
     }
   }
   throw Napi::TypeError::New(info.Env(),
-                             "No constructor with the given " + std::to_string(info.Length()) + " arguments found");
+                             "No constructor with the given "s + std::to_string(info.Length()) + " arguments found"s);
 }
 
 template <typename CLASS>
@@ -412,7 +414,7 @@ template <typename CLASS> inline CLASS *NoObjectWrap<CLASS>::CheckUnwrap(Napi::V
   Napi::Object obj = val.ToObject();
   auto instance = env.GetInstanceData<BaseEnvInstanceData>();
   if (!obj.InstanceOf(instance->_Nobind_cons[class_idx].Value())) {
-    throw Napi::TypeError::New(env, "Expected a " + name);
+    throw Napi::TypeError::New(env, "Expected a "s + (name.size() > 0 ? name : "<unknown to nobind17 class>"s));
   }
   return NoObjectWrap<CLASS>::Unwrap(obj)->self;
 }