[Pkg-javascript-commits] [node-leveldown] 338/492: batch memory leak, not disposing of String->Slice

[Andrew Kelley]

This is an automated email from the git hooks/post-receive script.

andrewrk-guest pushed a commit to annotated tag rocksdb-0.10.1
in repository node-leveldown.

commit e300d6ff9569de39d9227f0bda2b923b0b7d68fc
Author: Rod Vagg <rod at vagg.org>
Date:   Fri May 17 12:21:17 2013 +1000

    batch memory leak, not disposing of String->Slice
---
 src/batch.cc          | 26 +++++++++++++-------------
 src/batch.h           |  2 +-
 src/database.cc       | 29 +++++++++++++++--------------
 src/database.h        | 14 +++++++++++---
 src/database_async.cc |  2 +-
 src/database_async.h  |  4 ++--
 6 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/src/batch.cc b/src/batch.cc
index 7e26fab..2b09454 100644
--- a/src/batch.cc
+++ b/src/batch.cc
@@ -13,7 +13,7 @@ Batch::Batch (leveldown::Database* database, bool sync) : database(database) {
   options = new leveldb::WriteOptions();
   options->sync = sync;
   batch = new leveldb::WriteBatch();
-  references = new std::vector< v8::Persistent<v8::Value> >;
+  references = new std::vector<Reference>;
   hasData = false;
 }
 
@@ -103,14 +103,14 @@ v8::Handle<v8::Value> Batch::Put (const v8::Arguments& args) {
   LD_STRING_OR_BUFFER_TO_SLICE(key, keyBuffer, key)
   LD_STRING_OR_BUFFER_TO_SLICE(value, valueBuffer, value)
 
-  if (node::Buffer::HasInstance(keyBuffer->ToObject()))
-    batch->references->push_back(v8::Persistent<v8::Value>::New(
-        LD_NODE_ISOLATE_PRE
-        keyBuffer));
-  if (node::Buffer::HasInstance(valueBuffer->ToObject()))
-    batch->references->push_back(v8::Persistent<v8::Value>::New(
-        LD_NODE_ISOLATE_PRE
-        valueBuffer));
+  batch->references->push_back(Reference(
+      v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE keyBuffer)
+    , key
+  ));
+  batch->references->push_back(Reference(
+      v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE valueBuffer)
+    , value
+  ));
 
   batch->batch->Put(key, value);
   if (!batch->hasData)
@@ -130,10 +130,10 @@ v8::Handle<v8::Value> Batch::Del (const v8::Arguments& args) {
   v8::Local<v8::Value> keyBuffer = args[0];
   LD_STRING_OR_BUFFER_TO_SLICE(key, keyBuffer, key)
 
-  if (node::Buffer::HasInstance(keyBuffer->ToObject()))
-    batch->references->push_back(v8::Persistent<v8::Value>::New(
-        LD_NODE_ISOLATE_PRE
-        keyBuffer));
+  batch->references->push_back(Reference(
+      v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE keyBuffer)
+    , key
+  ));
 
   batch->batch->Delete(key);
   if (!batch->hasData)
diff --git a/src/batch.h b/src/batch.h
index 54ab7dc..dc07ce4 100644
--- a/src/batch.h
+++ b/src/batch.h
@@ -26,7 +26,7 @@ private:
   leveldown::Database* database;
   leveldb::WriteOptions* options;
   leveldb::WriteBatch* batch;
-  std::vector< v8::Persistent<v8::Value> >* references;
+  std::vector<Reference>* references;
   bool hasData; // keep track of whether we're writing data or not
 
   static v8::Persistent<v8::Function> constructor;
diff --git a/src/database.cc b/src/database.cc
index 7d2f98d..4b4dde3 100644
--- a/src/database.cc
+++ b/src/database.cc
@@ -411,8 +411,7 @@ v8::Handle<v8::Value> Database::Batch (const v8::Arguments& args) {
 
   v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(args[0]);
 
-  std::vector< v8::Persistent<v8::Value> >* references =
-      new std::vector< v8::Persistent<v8::Value> >;
+  std::vector< Reference >* references = new std::vector< Reference >;
   leveldb::WriteBatch* batch = new leveldb::WriteBatch();
   bool hasData = false;
 
@@ -433,10 +432,11 @@ v8::Handle<v8::Value> Database::Batch (const v8::Arguments& args) {
       batch->Delete(key);
       if (!hasData)
         hasData = true;
-      if (node::Buffer::HasInstance(keyBuffer->ToObject()))
-        references->push_back(v8::Persistent<v8::Value>::New(
-            LD_NODE_ISOLATE_PRE
-            keyBuffer));
+
+      references->push_back(Reference(
+          v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE keyBuffer)
+        , key
+      ));
     } else if (obj->Get(str_type)->StrictEquals(str_put)) {
       v8::Local<v8::Value> valueBuffer = obj->Get(str_value);
       LD_CB_ERR_IF_NULL_OR_UNDEFINED(valueBuffer, value)
@@ -447,14 +447,15 @@ v8::Handle<v8::Value> Database::Batch (const v8::Arguments& args) {
       batch->Put(key, value);
       if (!hasData)
         hasData = true;
-      if (node::Buffer::HasInstance(keyBuffer->ToObject()))
-        references->push_back(v8::Persistent<v8::Value>::New(
-            LD_NODE_ISOLATE_PRE
-            keyBuffer));
-      if (node::Buffer::HasInstance(valueBuffer->ToObject()))
-        references->push_back(v8::Persistent<v8::Value>::New(
-            LD_NODE_ISOLATE_PRE
-            valueBuffer));
+
+      references->push_back(Reference(
+          v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE keyBuffer)
+        , key
+      ));
+      references->push_back(Reference(
+          v8::Persistent<v8::Value>::New(LD_NODE_ISOLATE_PRE valueBuffer)
+        , value
+      ));
     }
   }
 
diff --git a/src/database.h b/src/database.h
index 25e8019..5a5a32a 100644
--- a/src/database.h
+++ b/src/database.h
@@ -32,11 +32,19 @@ struct AsyncDescriptor;
 
 v8::Handle<v8::Value> LevelDOWN (const v8::Arguments& args);
 
-static inline void ClearReferences (std::vector< v8::Persistent<v8::Value> >* references) {
-  for (std::vector< v8::Persistent<v8::Value> >::iterator it = references->begin()
+struct Reference {
+  v8::Persistent<v8::Value> ptr;
+  leveldb::Slice slice;
+  Reference(v8::Persistent<v8::Value> ptr, leveldb::Slice slice) :
+      ptr(ptr)
+    , slice(slice) { };
+};
+
+static inline void ClearReferences (std::vector<Reference>* references) {
+  for (std::vector<Reference>::iterator it = references->begin()
       ; it != references->end()
       ; ) {
-    it->Dispose(LD_NODE_ISOLATE);
+    DisposeStringOrBufferFromSlice(it->ptr, it->slice);
     it = references->erase(it);
   }
   delete references;
diff --git a/src/database_async.cc b/src/database_async.cc
index a6e6ad9..ef24498 100644
--- a/src/database_async.cc
+++ b/src/database_async.cc
@@ -181,7 +181,7 @@ BatchWorker::BatchWorker (
     Database* database
   , v8::Persistent<v8::Function> callback
   , leveldb::WriteBatch* batch
-  , std::vector< v8::Persistent<v8::Value> >* references
+  , std::vector<Reference>* references
   , bool sync
 ) : AsyncWorker(database, callback)
   , batch(batch)
diff --git a/src/database_async.h b/src/database_async.h
index 8ca9fdd..80dafff 100644
--- a/src/database_async.h
+++ b/src/database_async.h
@@ -131,7 +131,7 @@ public:
       Database* database
     , v8::Persistent<v8::Function> callback
     , leveldb::WriteBatch* batch
-    , std::vector< v8::Persistent<v8::Value> >* references
+    , std::vector<Reference>* references
     , bool sync
   );
 
@@ -141,7 +141,7 @@ public:
 private:
   leveldb::WriteOptions* options;
   leveldb::WriteBatch* batch;
-  std::vector< v8::Persistent<v8::Value> >* references;
+  std::vector<Reference>* references;
 };
 
 class ApproximateSizeWorker : public AsyncWorker {

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/node-leveldown.git