Skip to content
This repository was archived by the owner on Sep 23, 2024. It is now read-only.

Commit 712c777

Browse files
guymguymguymguym
committed
fix memory leak of buffers and other persistent handles
also replaced the CopyBuffer in BSONSerialize to NewBuffer which avoids another unneeded memory allocation, since the used memory was just been allocated.
1 parent 6e86102 commit 712c777

File tree

2 files changed

+126
-77
lines changed

2 files changed

+126
-77
lines changed

Diff for: ‎ext/bson.cc

+93-45
Original file line numberDiff line numberDiff line change
@@ -598,39 +598,56 @@ Local<Value> BSONDeserializer::DeserializeValue(BsonType type, bool promoteLongs
598598
return Nan::Null();
599599
}
600600

601+
// statics
601602
Persistent<FunctionTemplate> BSON::constructor_template;
603+
Persistent<String> BSON::longString;
604+
Persistent<String> BSON::objectIDString;
605+
Persistent<String> BSON::binaryString;
606+
Persistent<String> BSON::codeString;
607+
Persistent<String> BSON::dbrefString;
608+
Persistent<String> BSON::symbolString;
609+
Persistent<String> BSON::doubleString;
610+
Persistent<String> BSON::timestampString;
611+
Persistent<String> BSON::minKeyString;
612+
Persistent<String> BSON::maxKeyString;
613+
Persistent<String> BSON::_bsontypeString;
614+
Persistent<String> BSON::_longLowString;
615+
Persistent<String> BSON::_longHighString;
616+
Persistent<String> BSON::_objectIDidString;
617+
Persistent<String> BSON::_binaryPositionString;
618+
Persistent<String> BSON::_binarySubTypeString;
619+
Persistent<String> BSON::_binaryBufferString;
620+
Persistent<String> BSON::_doubleValueString;
621+
Persistent<String> BSON::_symbolValueString;
622+
Persistent<String> BSON::_dbRefRefString;
623+
Persistent<String> BSON::_dbRefIdRefString;
624+
Persistent<String> BSON::_dbRefDbRefString;
625+
Persistent<String> BSON::_dbRefNamespaceString;
626+
Persistent<String> BSON::_dbRefDbString;
627+
Persistent<String> BSON::_dbRefOidString;
628+
Persistent<String> BSON::_codeCodeString;
629+
Persistent<String> BSON::_codeScopeString;
630+
Persistent<String> BSON::_toBSONString;
631+
632+
BSON::BSON() : ObjectWrap()
633+
{
634+
}
602635

603-
BSON::BSON() : ObjectWrap() {
604-
// Setup pre-allocated comparision objects
605-
NanAssignPersistent(_bsontypeString, NanStr("_bsontype"));
606-
NanAssignPersistent(_longLowString, NanStr("low_"));
607-
NanAssignPersistent(_longHighString, NanStr("high_"));
608-
NanAssignPersistent(_objectIDidString, NanStr("id"));
609-
NanAssignPersistent(_binaryPositionString, NanStr("position"));
610-
NanAssignPersistent(_binarySubTypeString, NanStr("sub_type"));
611-
NanAssignPersistent(_binaryBufferString, NanStr("buffer"));
612-
NanAssignPersistent(_doubleValueString, NanStr("value"));
613-
NanAssignPersistent(_symbolValueString, NanStr("value"));
614-
NanAssignPersistent(_dbRefRefString, NanStr("$ref"));
615-
NanAssignPersistent(_dbRefIdRefString, NanStr("$id"));
616-
NanAssignPersistent(_dbRefDbRefString, NanStr("$db"));
617-
NanAssignPersistent(_dbRefNamespaceString, NanStr("namespace"));
618-
NanAssignPersistent(_dbRefDbString, NanStr("db"));
619-
NanAssignPersistent(_dbRefOidString, NanStr("oid"));
620-
NanAssignPersistent(_codeCodeString, NanStr("code"));
621-
NanAssignPersistent(_codeScopeString, NanStr("scope"));
622-
NanAssignPersistent(_toBSONString, NanStr("toBSON"));
623-
624-
NanAssignPersistent(longString, NanStr("Long"));
625-
NanAssignPersistent(objectIDString, NanStr("ObjectID"));
626-
NanAssignPersistent(binaryString, NanStr("Binary"));
627-
NanAssignPersistent(codeString, NanStr("Code"));
628-
NanAssignPersistent(dbrefString, NanStr("DBRef"));
629-
NanAssignPersistent(symbolString, NanStr("Symbol"));
630-
NanAssignPersistent(doubleString, NanStr("Double"));
631-
NanAssignPersistent(timestampString, NanStr("Timestamp"));
632-
NanAssignPersistent(minKeyString, NanStr("MinKey"));
633-
NanAssignPersistent(maxKeyString, NanStr("MaxKey"));
636+
BSON::~BSON()
637+
{
638+
Nan::HandleScope scope;
639+
// dispose persistent handles
640+
buffer.Reset();
641+
longConstructor.Reset();
642+
objectIDConstructor.Reset();
643+
binaryConstructor.Reset();
644+
codeConstructor.Reset();
645+
dbrefConstructor.Reset();
646+
symbolConstructor.Reset();
647+
doubleConstructor.Reset();
648+
timestampConstructor.Reset();
649+
minKeyConstructor.Reset();
650+
maxKeyConstructor.Reset();
634651
}
635652

636653
void BSON::Initialize(v8::Local<v8::Object> target) {
@@ -648,9 +665,40 @@ void BSON::Initialize(v8::Local<v8::Object> target) {
648665
Nan::SetPrototypeMethod(t, "deserialize", BSONDeserialize);
649666
Nan::SetPrototypeMethod(t, "deserializeStream", BSONDeserializeStream);
650667

651-
NanAssignPersistent(constructor_template, t);
668+
constructor_template.Reset(t);
669+
670+
// Setup pre-allocated comparision objects
671+
_bsontypeString.Reset(NanStr("_bsontype"));
672+
_longLowString.Reset(NanStr("low_"));
673+
_longHighString.Reset(NanStr("high_"));
674+
_objectIDidString.Reset(NanStr("id"));
675+
_binaryPositionString.Reset(NanStr("position"));
676+
_binarySubTypeString.Reset(NanStr("sub_type"));
677+
_binaryBufferString.Reset(NanStr("buffer"));
678+
_doubleValueString.Reset(NanStr("value"));
679+
_symbolValueString.Reset(NanStr("value"));
680+
_dbRefRefString.Reset(NanStr("$ref"));
681+
_dbRefIdRefString.Reset(NanStr("$id"));
682+
_dbRefDbRefString.Reset(NanStr("$db"));
683+
_dbRefNamespaceString.Reset(NanStr("namespace"));
684+
_dbRefDbString.Reset(NanStr("db"));
685+
_dbRefOidString.Reset(NanStr("oid"));
686+
_codeCodeString.Reset(NanStr("code"));
687+
_codeScopeString.Reset(NanStr("scope"));
688+
_toBSONString.Reset(NanStr("toBSON"));
689+
longString.Reset(NanStr("Long"));
690+
objectIDString.Reset(NanStr("ObjectID"));
691+
binaryString.Reset(NanStr("Binary"));
692+
codeString.Reset(NanStr("Code"));
693+
dbrefString.Reset(NanStr("DBRef"));
694+
symbolString.Reset(NanStr("Symbol"));
695+
doubleString.Reset(NanStr("Double"));
696+
timestampString.Reset(NanStr("Timestamp"));
697+
minKeyString.Reset(NanStr("MinKey"));
698+
maxKeyString.Reset(NanStr("MaxKey"));
652699

653700
target->ForceSet(NanStr("BSON"), t->GetFunction());
701+
654702
}
655703

656704
// Create a new instance of BSON and passing it the existing context
@@ -681,7 +729,7 @@ NAN_METHOD(BSON::New) {
681729
bson->maxBSONSize = maxBSONSize;
682730

683731
// Allocate a new Buffer
684-
NanAssignPersistent(bson->buffer, Unmaybe(Nan::NewBuffer(sizeof(char) * maxBSONSize)));
732+
bson->buffer.Reset(Unmaybe(Nan::NewBuffer(sizeof(char) * maxBSONSize)));
685733

686734
// Defined the classmask
687735
uint32_t foundClassesMask = 0;
@@ -694,34 +742,34 @@ NAN_METHOD(BSON::New) {
694742

695743
// Save the functions making them persistant handles (they don't get collected)
696744
if(functionName->StrictEquals(NanStr(bson->longString))) {
697-
NanAssignPersistent(bson->longConstructor, func);
745+
bson->longConstructor.Reset(func);
698746
foundClassesMask |= 1;
699747
} else if(functionName->StrictEquals(NanStr(bson->objectIDString))) {
700-
NanAssignPersistent(bson->objectIDConstructor, func);
748+
bson->objectIDConstructor.Reset(func);
701749
foundClassesMask |= 2;
702750
} else if(functionName->StrictEquals(NanStr(bson->binaryString))) {
703-
NanAssignPersistent(bson->binaryConstructor, func);
751+
bson->binaryConstructor.Reset(func);
704752
foundClassesMask |= 4;
705753
} else if(functionName->StrictEquals(NanStr(bson->codeString))) {
706-
NanAssignPersistent(bson->codeConstructor, func);
754+
bson->codeConstructor.Reset(func);
707755
foundClassesMask |= 8;
708756
} else if(functionName->StrictEquals(NanStr(bson->dbrefString))) {
709-
NanAssignPersistent(bson->dbrefConstructor, func);
757+
bson->dbrefConstructor.Reset(func);
710758
foundClassesMask |= 0x10;
711759
} else if(functionName->StrictEquals(NanStr(bson->symbolString))) {
712-
NanAssignPersistent(bson->symbolConstructor, func);
760+
bson->symbolConstructor.Reset(func);
713761
foundClassesMask |= 0x20;
714762
} else if(functionName->StrictEquals(NanStr(bson->doubleString))) {
715-
NanAssignPersistent(bson->doubleConstructor, func);
763+
bson->doubleConstructor.Reset(func);
716764
foundClassesMask |= 0x40;
717765
} else if(functionName->StrictEquals(NanStr(bson->timestampString))) {
718-
NanAssignPersistent(bson->timestampConstructor, func);
766+
bson->timestampConstructor.Reset(func);
719767
foundClassesMask |= 0x80;
720768
} else if(functionName->StrictEquals(NanStr(bson->minKeyString))) {
721-
NanAssignPersistent(bson->minKeyConstructor, func);
769+
bson->minKeyConstructor.Reset(func);
722770
foundClassesMask |= 0x100;
723771
} else if(functionName->StrictEquals(NanStr(bson->maxKeyString))) {
724-
NanAssignPersistent(bson->maxKeyConstructor, func);
772+
bson->maxKeyConstructor.Reset(func);
725773
foundClassesMask |= 0x200;
726774
}
727775
}
@@ -895,8 +943,8 @@ NAN_METHOD(BSON::BSONSerialize) {
895943

896944
// If we have 3 arguments
897945
if(info.Length() == 3 || info.Length() == 4) {
898-
Local<Object> buffer = Unmaybe(Nan::CopyBuffer(serialized_object, object_size));
899-
free(final);
946+
// NewBuffer takes ownership on the memory, so no need to free the final allocation
947+
Local<Object> buffer = Unmaybe(Nan::NewBuffer(serialized_object, object_size));
900948
info.GetReturnValue().Set(buffer);
901949
} else {
902950
Local<Value> bin_value = Nan::Encode(serialized_object, object_size)->ToString();

Diff for: ‎ext/bson.h

+33-32
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,19 @@ using Nan::ObjectWrap;
3535
#define NanStr(x) (Unmaybe(Nan::New<String>(x)))
3636
#define NanHas(obj, key) (Nan::Has(obj, NanKey(key)).FromJust())
3737
#define NanGet(obj, key) (Unmaybe(Nan::Get(obj, NanKey(key))))
38-
#define NanAssignPersistent(persistent, value) persistent.Reset(value)
3938
// Unmaybe overloading to conviniently convert from Local/MaybeLocal/Maybe to Local/plain value
4039
template <class T>
4140
inline Local<T> Unmaybe(Local<T> h) {
4241
return h;
4342
}
4443
template <class T>
4544
inline Local<T> Unmaybe(Nan::MaybeLocal<T> h) {
45+
assert(!h.IsEmpty());
4646
return h.ToLocalChecked();
4747
}
4848
template <class T>
4949
inline T Unmaybe(Nan::Maybe<T> h) {
50+
assert(h.IsJust());
5051
return h.FromJust();
5152
}
5253
// NanKey overloading to conviniently convert to a propert key for object/array
@@ -98,7 +99,7 @@ template<typename T> class BSONSerializer;
9899
class BSON : public Nan::ObjectWrap {
99100
public:
100101
BSON();
101-
~BSON() {}
102+
~BSON();
102103

103104
static void Initialize(Local<Object> target);
104105
static NAN_METHOD(BSONDeserializeStream);
@@ -135,38 +136,38 @@ class BSON : public Nan::ObjectWrap {
135136
Persistent<Function> maxKeyConstructor;
136137

137138
// Equality Objects
138-
Persistent<String> longString;
139-
Persistent<String> objectIDString;
140-
Persistent<String> binaryString;
141-
Persistent<String> codeString;
142-
Persistent<String> dbrefString;
143-
Persistent<String> symbolString;
144-
Persistent<String> doubleString;
145-
Persistent<String> timestampString;
146-
Persistent<String> minKeyString;
147-
Persistent<String> maxKeyString;
139+
static Persistent<String> longString;
140+
static Persistent<String> objectIDString;
141+
static Persistent<String> binaryString;
142+
static Persistent<String> codeString;
143+
static Persistent<String> dbrefString;
144+
static Persistent<String> symbolString;
145+
static Persistent<String> doubleString;
146+
static Persistent<String> timestampString;
147+
static Persistent<String> minKeyString;
148+
static Persistent<String> maxKeyString;
148149

149150
// Equality speed up comparison objects
150-
Persistent<String> _bsontypeString;
151-
Persistent<String> _longLowString;
152-
Persistent<String> _longHighString;
153-
Persistent<String> _objectIDidString;
154-
Persistent<String> _binaryPositionString;
155-
Persistent<String> _binarySubTypeString;
156-
Persistent<String> _binaryBufferString;
157-
Persistent<String> _doubleValueString;
158-
Persistent<String> _symbolValueString;
159-
160-
Persistent<String> _dbRefRefString;
161-
Persistent<String> _dbRefIdRefString;
162-
Persistent<String> _dbRefDbRefString;
163-
Persistent<String> _dbRefNamespaceString;
164-
Persistent<String> _dbRefDbString;
165-
Persistent<String> _dbRefOidString;
166-
167-
Persistent<String> _codeCodeString;
168-
Persistent<String> _codeScopeString;
169-
Persistent<String> _toBSONString;
151+
static Persistent<String> _bsontypeString;
152+
static Persistent<String> _longLowString;
153+
static Persistent<String> _longHighString;
154+
static Persistent<String> _objectIDidString;
155+
static Persistent<String> _binaryPositionString;
156+
static Persistent<String> _binarySubTypeString;
157+
static Persistent<String> _binaryBufferString;
158+
static Persistent<String> _doubleValueString;
159+
static Persistent<String> _symbolValueString;
160+
161+
static Persistent<String> _dbRefRefString;
162+
static Persistent<String> _dbRefIdRefString;
163+
static Persistent<String> _dbRefDbRefString;
164+
static Persistent<String> _dbRefNamespaceString;
165+
static Persistent<String> _dbRefDbString;
166+
static Persistent<String> _dbRefOidString;
167+
168+
static Persistent<String> _codeCodeString;
169+
static Persistent<String> _codeScopeString;
170+
static Persistent<String> _toBSONString;
170171

171172
Local<Object> GetSerializeObject(const Local<Value>& object);
172173

0 commit comments

Comments
 (0)