From 07522646204d121609655d30dabdee28934384f7 Mon Sep 17 00:00:00 2001
From: Sam Saffron <sam.saffron@gmail.com>
Date: Wed, 8 Dec 2021 16:17:36 +1100
Subject: [PATCH] rb_heap_snapshot: fix memory leak from fdopen

The result of fdopen(3) must be matched with fclose(3) to
avoid memory leaks.  However, fclose(3) has the side-effect
of closing the underlying FD behind Ruby's back (which would
corrupt Ruby's internals and cause problems for the remainder
of the process lifetime).

While one could dup(fptr->fd) to fdopen+fclose the resulting fd,
it would cause problems on FD-constrained systems.  Since
GetChunkSize() is available and returns a reasonable value
(64K), the buffering of fwrite(3) is unnecessary and we can
safely use write(2) without performance loss.

So use unbuffered write(2) and perhaps prepare us for better
error reporting in a future change.

I've verified WriteAsciiChunk is indeed receiving the requested
64K (or close to it) in nearly all cases, so excessive syscalls
should not be a problem.
---
 .../mini_racer_extension.cc                   | 40 ++++++++++++-------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/ext/mini_racer_extension/mini_racer_extension.cc b/ext/mini_racer_extension/mini_racer_extension.cc
index 878fc8d..6ba7f5c 100644
--- a/ext/mini_racer_extension/mini_racer_extension.cc
+++ b/ext/mini_racer_extension/mini_racer_extension.cc
@@ -11,6 +11,7 @@
 #include <mutex>
 #include <atomic>
 #include <math.h>
+#include <errno.h>
 
 using namespace v8;
 
@@ -1599,7 +1600,9 @@ rb_heap_stats(VALUE self) {
 // https://github.com/bnoordhuis/node-heapdump/blob/master/src/heapdump.cc
 class FileOutputStream : public OutputStream {
  public:
-  FileOutputStream(FILE* stream) : stream_(stream) {}
+  int err;
+
+  FileOutputStream(int fd) : fd(fd) { err = 0; }
 
   virtual int GetChunkSize() {
     return 65536;
@@ -1608,17 +1611,27 @@ class FileOutputStream : public OutputStream {
   virtual void EndOfStream() {}
 
   virtual WriteResult WriteAsciiChunk(char* data, int size) {
-    const size_t len = static_cast<size_t>(size);
-    size_t off = 0;
+    size_t len = static_cast<size_t>(size);
 
-    while (off < len && !feof(stream_) && !ferror(stream_))
-      off += fwrite(data + off, 1, len - off, stream_);
+    while (len) {
+        ssize_t w = write(fd, data, len);
 
-    return off == len ? kContinue : kAbort;
+        if (w > 0) {
+            data += w;
+            len -= w;
+        } else if (w < 0) {
+            err = errno;
+            return kAbort;
+        } else { /* w == 0, could be out-of-space */
+            err = -1;
+            return kAbort;
+        }
+    }
+    return kContinue;
   }
 
  private:
-  FILE* stream_;
+  int fd;
 };
 
 
@@ -1631,10 +1644,8 @@ rb_heap_snapshot(VALUE self, VALUE file) {
 
     if (!fptr) return Qfalse;
 
-    FILE* fp;
-    fp = fdopen(fptr->fd, "w");
-    if (fp == NULL) return Qfalse;
-
+    // prepare for unbuffered write(2) below:
+    rb_funcall(file, rb_intern("flush"), 0);
 
     ContextInfo* context_info;
     TypedData_Get_Struct(self, ContextInfo, &context_type, context_info);
@@ -1651,13 +1662,14 @@ rb_heap_snapshot(VALUE self, VALUE file) {
 
     const HeapSnapshot* const snap = heap_profiler->TakeHeapSnapshot();
 
-    FileOutputStream stream(fp);
+    FileOutputStream stream(fptr->fd);
     snap->Serialize(&stream, HeapSnapshot::kJSON);
 
-    fflush(fp);
-
     const_cast<HeapSnapshot*>(snap)->Delete();
 
+    /* TODO: perhaps rb_sys_fail here */
+    if (stream.err) return Qfalse;
+
     return Qtrue;
 }