Safe constructor for ObjectWrapped classes

bnoordhuis · ry · commit 55c65cc2d55b · 2010-09-29T16:02:45.000-07:00
New() methods should be invoked as constructors, not regular functions. Corner cases like Script::New() may cause a SIGSEGV when the GC is run. More details: http://groups.google.com/group/nodejs/browse_thread/thread/a7e5db68d4cd6356
diff --git a/src/node.cc b/src/node.cc
@@ -787,6 +787,22 @@ Local<Value> ErrnoException(int errorno,
 }
 
 
+Handle<Value> FromConstructorTemplate(Persistent<FunctionTemplate>& t,
+                                      const Arguments& args) {
+  HandleScope scope;
+
+  const int argc = args.Length();
+  Local<Value> argv[argc];
+
+  for (int i = 0; i < argc; ++i) {
+    argv[i] = args[i];
+  }
+
+  Local<Object> instance = t->GetFunction()->NewInstance(argc, argv);
+  return scope.Close(instance);
+}
+
+
 enum encoding ParseEncoding(Handle<Value> encoding_v, enum encoding _default) {
   HandleScope scope;
 
diff --git a/src/node.h b/src/node.h
@@ -65,6 +65,15 @@ ssize_t DecodeWrite(char *buf,
 v8::Local<v8::Object> BuildStatsObject(struct stat * s);
 
 
+/**
+ * Call this when your constructor is invoked as a regular function, e.g. Buffer(10) instead of new Buffer(10).
+ * @param constructorTemplate Constructor template to instantiate from.
+ * @param args The arguments object passed to your constructor.
+ * @see v8::Arguments::IsConstructCall
+ */
+v8::Handle<v8::Value> FromConstructorTemplate(v8::Persistent<v8::FunctionTemplate>& constructorTemplate, const v8::Arguments& args);
+
+
 static inline v8::Persistent<v8::Function>* cb_persist(
     const v8::Local<v8::Value> &v) {
   v8::Persistent<v8::Function> *fn = new v8::Persistent<v8::Function>();
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -127,18 +127,12 @@ size_t Buffer::Length(Handle<Object> obj) {
 
 
 Handle<Value> Buffer::New(const Arguments &args) {
-  HandleScope scope;
-
   if (!args.IsConstructCall()) {
-    Local<Value> argv[10];
-    for (int i = 0; i < MIN(args.Length(), 10); i++) {
-      argv[i] = args[i];
-    }
-    Local<Object> instance =
-      constructor_template->GetFunction()->NewInstance(args.Length(), argv);
-    return scope.Close(instance);
+    return FromConstructorTemplate(constructor_template, args);
   }
 
+  HandleScope scope;
+
   Buffer *buffer;
   if (args[0]->IsInt32()) {
     // var buffer = new Buffer(1024);
diff --git a/src/node_cares.cc b/src/node_cares.cc
@@ -459,6 +459,10 @@ void Channel::Initialize(Handle<Object> target) {
 
 
 Handle<Value> Channel::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   struct ares_options options;
diff --git a/src/node_idle_watcher.cc b/src/node_idle_watcher.cc
@@ -77,6 +77,10 @@ void IdleWatcher::Callback(EV_P_ ev_idle *w, int revents) {
 //  idle.start();
 //
 Handle<Value> IdleWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   IdleWatcher *s = new IdleWatcher();
diff --git a/src/node_io_watcher.cc b/src/node_io_watcher.cc
@@ -68,6 +68,10 @@ void IOWatcher::Callback(EV_P_ ev_io *w, int revents) {
 //  io.start();
 //
 Handle<Value> IOWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
   IOWatcher *s = new IOWatcher();
   s->Wrap(args.This());
diff --git a/src/node_script.cc b/src/node_script.cc
@@ -78,6 +78,10 @@ void node::Script::Initialize (Handle<Object> target) {
 
 
 Handle<Value> node::Script::New (const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   node::Script *t = new node::Script();
diff --git a/src/node_signal_watcher.cc b/src/node_signal_watcher.cc
@@ -51,6 +51,10 @@ void SignalWatcher::Callback(EV_P_ ev_signal *watcher, int revents) {
 }
 
 Handle<Value> SignalWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   if (args.Length() != 1 || !args[0]->IsInt32()) {
diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc
@@ -46,6 +46,10 @@ void StatWatcher::Callback(EV_P_ ev_stat *watcher, int revents) {
 
 
 Handle<Value> StatWatcher::New(const Arguments& args) {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
   StatWatcher *s = new StatWatcher();
   s->Wrap(args.Holder());
diff --git a/src/node_timer.cc b/src/node_timer.cc
@@ -97,6 +97,10 @@ Timer::~Timer ()
 Handle<Value>
 Timer::New (const Arguments& args)
 {
+  if (!args.IsConstructCall()) {
+    return FromConstructorTemplate(constructor_template, args);
+  }
+
   HandleScope scope;
 
   Timer *t = new Timer();

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,10 @@ void SignalWatcher::Callback(EV_P_ ev_signal *watcher, int revents) {`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`Handle<Value> SignalWatcher::New(const Arguments& args) {`
	`54`	`+ if (!args.IsConstructCall()) {`
	`55`	`+ return FromConstructorTemplate(constructor_template, args);`
	`56`	`+ }`
	`57`	`+`
`54`	`58`	`HandleScope scope;`
`55`	`59`
`56`	`60`	`if (args.Length() != 1 \|\| !args[0]->IsInt32()) {`