node-raylib icon indicating copy to clipboard operation
node-raylib copied to clipboard

stringFromValue is leaking memory

Open cunev opened this issue 4 months ago • 2 comments

inline const char * stringFromValue(const Napi::CallbackInfo& info, int index) {
  std::string val = info[index].As<Napi::String>().Utf8Value();
  const std::string::size_type size = val.size();
  char *buffer = new char[size + 1];   //we need extra char for NUL
  memcpy(buffer, val.c_str(), size + 1);
  return buffer;
}

Caused by this function, where it clearly allocates memory on char *buffer = new char[size + 1];. I noticed this when my game's memory was growing and I debugged it down to DrawTextEx call.

cunev avatar Sep 14 '25 17:09 cunev

Likely beat to use TextFormat() here so that raylib manages the string memory.

RobLoach avatar Sep 15 '25 04:09 RobLoach

Hi friends --

I encountered this recently myself and found that this tweak in the bindings generator worked pretty nicely:

diff --git i/tools/generate_templates/node-raylib-bindings.js w/tools/generate_templates/node-raylib-bindings.js
index 68f90c9..fe8b3c5 100644
--- i/tools/generate_templates/node-raylib-bindings.js
+++ w/tools/generate_templates/node-raylib-bindings.js
@@ -65,11 +65,18 @@ const TypeUnwrappedLength = (structs, type) => {
   return properties
 }
 
+const TypeSuffix = (type) => {
+  if (type == "const char *") {
+    return ".c_str()";
+  }
+  return "";
+}
+
 const UnwrappedStructProperties = (structs, struct) => {
   let length = 0
   return struct.fields
     .map(field => {
-      const out = `${field.type.endsWith('*') ? ` (${field.type})` : ''} ${SanitizeTypeName(field.type)}FromValue(info, index + ${length})`
+      const out = `${field.type.endsWith('*') ? ` (${field.type})` : ''} ${SanitizeTypeName(field.type)}FromValue(info, index + ${length})${TypeSuffix(field.type)}`
       length += TypeUnwrappedLength(structs, field.type)
       return out
     })
@@ -82,7 +89,7 @@ const UnwrappedFuncArguments = (structs, func) => {
 
   return func.params
     .map(param => {
-      const out = `${param.type.endsWith('*') ? ` (${param.type})` : ''} ${SanitizeTypeName(param.type)}FromValue(info, ${length})`
+      const out = `${param.type.endsWith('*') ? ` (${param.type})` : ''} ${SanitizeTypeName(param.type)}FromValue(info, ${length})${TypeSuffix(param.type)}`
       length += TypeUnwrappedLength(structs, param.type)
       return out
     })
@@ -160,7 +167,7 @@ Napi::Value Bind${func.name}(const Napi::CallbackInfo& info) {
       : ['&obj'].concat(func.params
         ?.filter((param, index) => index !== 0)
         .map(param => {
-          const out = `${param.type.endsWith('*') ? ` (${param.type})` : ''} ${SanitizeTypeName(param.type)}FromValue(info, ${length})`
+          const out = `${param.type.endsWith('*') ? ` (${param.type})` : ''} ${SanitizeTypeName(param.type)}FromValue(info, ${length})${TypeSuffix(param.type)}`
           length += TypeUnwrappedLength(structs, param.type)
           return out
         })
@@ -253,12 +260,9 @@ inline unsigned long long unsignedlonglongFromValue(const Napi::CallbackInfo& in
 inline bool boolFromValue(const Napi::CallbackInfo& info, int index) {
   return info[index].As<Napi::Boolean>();
 }
-inline const char * stringFromValue(const Napi::CallbackInfo& info, int index) {
+inline std::string stringFromValue(const Napi::CallbackInfo& info, int index) {
   std::string val = info[index].As<Napi::String>().Utf8Value();
-  const std::string::size_type size = val.size();
-  char *buffer = new char[size + 1];   //we need extra char for NUL
-  memcpy(buffer, val.c_str(), size + 1);
-  return buffer;
+  return val;
 }
 inline char charFromValue(const Napi::CallbackInfo& info, int index) {
   return info[index].As<Napi::Number>().Uint32Value();

If you run npm run gen:code after applying this patch, it should regenerate node-raylib.cc, which changes bindings from this:

Napi::Value BindLoadShader(const Napi::CallbackInfo& info) {
  return ToValue(info.Env(),
    LoadShader(
       (const char *) stringFromValue(info, 0),
       (const char *) stringFromValue(info, 1)
    )
  );
}

to this:

Napi::Value BindLoadShader(const Napi::CallbackInfo& info) {
  return ToValue(info.Env(),
    LoadShader(
       (const char *) stringFromValue(info, 0).c_str(),
       (const char *) stringFromValue(info, 1).c_str()
    )
  );
}

This basically just uses RAII to auto-cleanup the std::string temporaries returned from stringFromValue() when the binding function returns, and seems to cleanly avoid a leak here without having to rewrite the whole bindings system. Feel free to make it your own!

joedrago avatar Oct 08 '25 23:10 joedrago