buffer: allocate memory with mmap()

bnoordhuis · bnoordhuis · commit 2433ec827683 · 2012-12-16T10:19:09.000+01:00
Work around an issue with the glibc malloc() implementation where memory blocks are never returned to the operating system when they are allocated with brk() and have overlapping lifecycles. Fixes nodejs#4283.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -20,6 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 var SlowBuffer = process.binding('buffer').SlowBuffer;
+var kPoolSize = process.binding('buffer').kPoolSize;
 var assert = require('assert');
 
 exports.INSPECT_MAX_BYTES = 50;
@@ -297,7 +298,7 @@ Buffer.isEncoding = function(encoding) {
 
 
 
-Buffer.poolSize = 8 * 1024;
+Buffer.poolSize = kPoolSize;  // see src/node_buffer.h
 var pool;
 
 function allocPool() {
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -29,6 +29,12 @@
 #include <assert.h>
 #include <string.h> // memcpy
 
+#ifdef __POSIX__
+# include <sys/mman.h>  // mmap
+# include <unistd.h>    // sysconf
+# include <stdio.h>     // perror
+#endif
+
 #define MIN(a,b) ((a) < (b) ? (a) : (b))
 
 #define BUFFER_CLASS_ID (0xBABE)
@@ -196,6 +202,67 @@ Buffer::~Buffer() {
 }
 
 
+#if defined(__POSIX__)
+
+static unsigned int num_pool_buffers;
+static char* cached_pool_buffers[16];
+
+
+static inline void free_buf_mem(char* buf, size_t len) {
+  if (len == Buffer::kPoolSize &&
+      num_pool_buffers < ARRAY_SIZE(cached_pool_buffers)) {
+    cached_pool_buffers[num_pool_buffers++] = buf;
+    return;
+  }
+
+  if (munmap(buf, len)) {
+    perror("munmap");
+    abort();
+  }
+}
+
+
+static inline char* alloc_buf_mem(size_t len) {
+  size_t pagesize = sysconf(_SC_PAGESIZE);
+
+  len = ROUND_UP(len, pagesize);
+  if (len == Buffer::kPoolSize && num_pool_buffers > 0) {
+    return cached_pool_buffers[--num_pool_buffers];
+  }
+
+  int prot = PROT_READ | PROT_WRITE;
+  int flags = MAP_ANONYMOUS | MAP_PRIVATE;
+  char* buf = static_cast<char*>(mmap(NULL, len, prot, flags, -1, 0));
+
+  if (buf == NULL) {
+    TryCatch try_catch;
+    char errmsg[256];
+    snprintf(errmsg,
+             sizeof(errmsg),
+             "Out of memory, mmap(len=%lu) failed.",
+             static_cast<unsigned long>(len));
+    ThrowError(errmsg);
+    FatalException(try_catch);
+    abort();
+  }
+
+  return buf;
+}
+
+#else  // !__POSIX__
+
+static inline void free_buf_mem(char* buf, size_t len) {
+  delete[] buf;
+}
+
+
+static inline char* alloc_buf_mem(size_t len) {
+  return new char[len];
+}
+
+#endif  // __POSIX__
+
+
 // if replace doesn't have a callback, data must be copied
 // const_cast in Buffer::New requires this
 void Buffer::Replace(char *data, size_t length,
@@ -205,7 +272,7 @@ void Buffer::Replace(char *data, size_t length,
   if (callback_) {
     callback_(data_, callback_hint_);
   } else if (length_) {
-    delete [] data_;
+    free_buf_mem(data_, length_);
     V8::AdjustAmountOfExternalAllocatedMemory(
         -static_cast<intptr_t>(sizeof(Buffer) + length_));
   }
@@ -217,9 +284,8 @@ void Buffer::Replace(char *data, size_t length,
   if (callback_) {
     data_ = data;
   } else if (length_) {
-    data_ = new char[length_];
-    if (data)
-      memcpy(data_, data, length_);
+    data_ = alloc_buf_mem(length_);
+    if (data != NULL) memcpy(data_, data, length_);
     V8::AdjustAmountOfExternalAllocatedMemory(sizeof(Buffer) + length_);
   } else {
     data_ = NULL;
@@ -830,6 +896,8 @@ void Buffer::Initialize(Handle<Object> target) {
                   Buffer::MakeFastBuffer);
 
   target->Set(String::NewSymbol("SlowBuffer"), constructor_template->GetFunction());
+  target->Set(String::NewSymbol("kPoolSize"),
+              Integer::NewFromUnsigned(kPoolSize));
 
   HeapProfiler::DefineWrapperClass(BUFFER_CLASS_ID, WrapperInfo);
 }
diff --git a/src/node_buffer.h b/src/node_buffer.h
@@ -68,6 +68,9 @@ class NODE_EXTERN Buffer: public ObjectWrap {
   // mirrors deps/v8/src/objects.h
   static const unsigned int kMaxLength = 0x3fffffff;
 
+  // exported in lib/buffer.js as Buffer.poolSize
+  static const unsigned int kPoolSize = 32768;
+
   static v8::Persistent<v8::FunctionTemplate> constructor_template;
 
   static bool HasInstance(v8::Handle<v8::Value> val);
diff --git a/test/simple/test-buffer-release.js b/test/simple/test-buffer-release.js
@@ -0,0 +1,37 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+function alloc() {
+  var h = {};
+  for (var i = 0; i < 10000; ++i) h[i] = new Buffer(20000);
+  h = null;
+}
+
+alloc();
+alloc();
+alloc();
+
+// Note: this assertion fails when run under valgrind because valgrind
+// increases the RSS footprint of node with at least 50 MB.
+assert(process.memoryUsage().rss < 32*1024*1024);
