buffer: switch API to return MaybeLocal<T>

Instead of aborting in case of internal failure, return an empty
Local<Object>. Using the MaybeLocal<T> API, users must check their
return values.

PR-URL: #1825
Reviewed-By: Ben Noordhuis <[email protected]>

Author: trevnorris

src/js_stream.cc

@@ -88,8 +88,11 @@ int JSStream::DoWrite(WriteWrap* w,
   HandleScope scope(env()->isolate());
 
   Local<Array> bufs_arr = Array::New(env()->isolate(), count);
-  for (size_t i = 0; i < count; i++)
-    bufs_arr->Set(i, Buffer::New(env(), bufs[i].base, bufs[i].len));
+  Local<Object> buf;
+  for (size_t i = 0; i < count; i++) {
+    buf = Buffer::New(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
+    bufs_arr->Set(i, buf);
+  }
 
   Local<Value> argv[] = {
     w->object(),
@@ -134,11 +137,13 @@ void JSStream::DoAlloc(const FunctionCallbackInfo<Value>& args) {
 
   uv_buf_t buf;
   wrap->OnAlloc(args[0]->Int32Value(), &buf);
-  args.GetReturnValue().Set(Buffer::New(wrap->env(),
-                                        buf.base,
-                                        buf.len,
-                                        FreeCallback,
-                                        nullptr));
+  Local<Object> vbuf = Buffer::New(
+      wrap->env(),
+      buf.base,
+      buf.len,
+      FreeCallback,
+      nullptr).ToLocalChecked();
+  return args.GetReturnValue().Set(vbuf);
 }
 
 

src/node_buffer.cc

@@ -74,6 +74,7 @@ using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::Maybe;
+using v8::MaybeLocal;
 using v8::Number;
 using v8::Object;
 using v8::Persistent;
@@ -234,7 +235,9 @@ size_t Length(Handle<Object> obj) {
 }
 
 
-Local<Object> New(Isolate* isolate, Handle<String> string, enum encoding enc) {
+MaybeLocal<Object> New(Isolate* isolate,
+                       Local<String> string,
+                       enum encoding enc) {
   EscapableHandleScope scope(isolate);
 
   size_t length = StringBytes::Size(isolate, string, enc);
@@ -251,19 +254,26 @@ Local<Object> New(Isolate* isolate, Handle<String> string, enum encoding enc) {
     CHECK_NE(data, nullptr);
   }
 
-  Local<Object> buf = Use(isolate, data, actual);
-  return scope.Escape(buf);
+  Local<Object> buf;
+  if (Use(isolate, data, actual).ToLocal(&buf))
+    return scope.Escape(buf);
+
+  // Object failed to be created. Clean up resources.
+  free(data);
+  return Local<Object>();
 }
 
 
-Local<Object> New(Isolate* isolate, size_t length) {
+MaybeLocal<Object> New(Isolate* isolate, size_t length) {
   EscapableHandleScope handle_scope(isolate);
-  Local<Object> obj = Buffer::New(Environment::GetCurrent(isolate), length);
-  return handle_scope.Escape(obj);
+  Local<Object> obj;
+  if (Buffer::New(Environment::GetCurrent(isolate), length).ToLocal(&obj))
+    return handle_scope.Escape(obj);
+  return Local<Object>();
 }
 
 
-Local<Object> New(Environment* env, size_t length) {
+MaybeLocal<Object> New(Environment* env, size_t length) {
   EscapableHandleScope scope(env->isolate());
 
   if (using_old_buffer) {
@@ -286,13 +296,12 @@ Local<Object> New(Environment* env, size_t length) {
   void* data;
   if (length > 0) {
     data = malloc(length);
-    // NOTE: API change. Must check .IsEmpty() on the return object to see if
-    // the data was able to be allocated.
     if (data == nullptr)
       return Local<Object>();
   } else {
     data = nullptr;
   }
+
   Local<ArrayBuffer> ab =
     ArrayBuffer::New(env->isolate(),
         data,
@@ -301,25 +310,27 @@ Local<Object> New(Environment* env, size_t length) {
   Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (!mb.FromMaybe(false)) {
-    FatalError("node::Buffer::New(Environment*, size_t)",
-               "Could not set Object prototype");
-    UNREACHABLE();
-  }
-  return scope.Escape(ui);
+  if (mb.FromMaybe(false))
+    return scope.Escape(ui);
+
+  // Object failed to be created. Clean up resources.
+  free(data);
+  return Local<Object>();
 }
 
 
-Local<Object> New(Isolate* isolate, const char* data, size_t length) {
+MaybeLocal<Object> New(Isolate* isolate, const char* data, size_t length) {
   Environment* env = Environment::GetCurrent(isolate);
   EscapableHandleScope handle_scope(env->isolate());
-  Local<Object> obj = Buffer::New(env, data, length);
-  return handle_scope.Escape(obj);
+  Local<Object> obj;
+  if (Buffer::New(env, data, length).ToLocal(&obj))
+    return handle_scope.Escape(obj);
+  return Local<Object>();
 }
 
 
 // Make a copy of "data". Why this isn't called "Copy", we'll never know.
-Local<Object> New(Environment* env, const char* data, size_t length) {
+MaybeLocal<Object> New(Environment* env, const char* data, size_t length) {
   EscapableHandleScope scope(env->isolate());
 
   if (using_old_buffer) {
@@ -353,8 +364,6 @@ Local<Object> New(Environment* env, const char* data, size_t length) {
   if (length > 0) {
     CHECK_NE(data, nullptr);
     new_data = malloc(length);
-    // NOTE: API change. Must check .IsEmpty() on the return object to see if
-    // the data was able to be allocated.
     if (new_data == nullptr)
       return Local<Object>();
     memcpy(new_data, data, length);
@@ -370,33 +379,34 @@ Local<Object> New(Environment* env, const char* data, size_t length) {
   Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (!mb.FromMaybe(false)) {
-    FatalError("node::Buffer::New(Environment*, char*, size_t)",
-               "Could not set Object prototype");
-    UNREACHABLE();
-  }
+  if (mb.FromMaybe(false))
+    return scope.Escape(ui);
 
-  return scope.Escape(ui);
+  // Object failed to be created. Clean up resources.
+  free(new_data);
+  return Local<Object>();
 }
 
 
-Local<Object> New(Isolate* isolate,
-                  char* data,
-                  size_t length,
-                  FreeCallback callback,
-                  void* hint) {
+MaybeLocal<Object> New(Isolate* isolate,
+                       char* data,
+                       size_t length,
+                       FreeCallback callback,
+                       void* hint) {
   Environment* env = Environment::GetCurrent(isolate);
   EscapableHandleScope handle_scope(env->isolate());
-  Local<Object> obj = Buffer::New(env, data, length, callback, hint);
-  return handle_scope.Escape(obj);
+  Local<Object> obj;
+  if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj))
+    return handle_scope.Escape(obj);
+  return Local<Object>();
 }
 
 
-Local<Object> New(Environment* env,
-                  char* data,
-                  size_t length,
-                  FreeCallback callback,
-                  void* hint) {
+MaybeLocal<Object> New(Environment* env,
+                       char* data,
+                       size_t length,
+                       FreeCallback callback,
+                       void* hint) {
   EscapableHandleScope scope(env->isolate());
 
   if (using_old_buffer) {
@@ -416,26 +426,26 @@ Local<Object> New(Environment* env,
   Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (!mb.FromMaybe(false)) {
-    FatalError("node::Buffer::New(Environment*, char*, size_t,"
-               " FreeCallback, void*)",
-               "Could not set Object prototype");
-    UNREACHABLE();
-  }
+
+  if (!mb.FromMaybe(false))
+    return Local<Object>();
+
   CallbackInfo::New(env->isolate(), ui, callback, hint);
   return scope.Escape(ui);
 }
 
 
-Local<Object> Use(Isolate* isolate, char* data, size_t length) {
+MaybeLocal<Object> Use(Isolate* isolate, char* data, size_t length) {
   Environment* env = Environment::GetCurrent(isolate);
   EscapableHandleScope handle_scope(env->isolate());
-  Local<Object> obj = Buffer::Use(env, data, length);
-  return handle_scope.Escape(obj);
+  Local<Object> obj;
+  if (Buffer::Use(env, data, length).ToLocal(&obj))
+    return handle_scope.Escape(obj);
+  return Local<Object>();
 }
 
 
-Local<Object> Use(Environment* env, char* data, size_t length) {
+MaybeLocal<Object> Use(Environment* env, char* data, size_t length) {
   EscapableHandleScope scope(env->isolate());
 
   if (using_old_buffer) {
@@ -463,12 +473,9 @@ Local<Object> Use(Environment* env, char* data, size_t length) {
   Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (!mb.FromMaybe(false)) {
-    FatalError("node::Buffer::Use(Environment*, char*, size_t)",
-               "Could not set Object prototype");
-    UNREACHABLE();
-  }
-  return scope.Escape(ui);
+  if (mb.FromMaybe(false))
+    return scope.Escape(ui);
+  return Local<Object>();
 }
 
 
@@ -501,8 +508,9 @@ void Create(const FunctionCallbackInfo<Value>& args) {
   Local<Uint8Array> ui = Uint8Array::New(ab, 0, length);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (mb.FromMaybe(false))
-    args.GetReturnValue().Set(ui);
+  if (!mb.FromMaybe(false))
+    return env->ThrowError("Unable to set Object prototype");
+  args.GetReturnValue().Set(ui);
 }
 
 
@@ -513,8 +521,9 @@ void CreateFromString(const FunctionCallbackInfo<Value>& args) {
   enum encoding enc = ParseEncoding(args.GetIsolate(),
                                     args[1].As<String>(),
                                     UTF8);
-  Local<Object> buf = New(args.GetIsolate(), args[0].As<String>(), enc);
-  args.GetReturnValue().Set(buf);
+  Local<Object> buf;
+  if (New(args.GetIsolate(), args[0].As<String>(), enc).ToLocal(&buf))
+    args.GetReturnValue().Set(buf);
 }
 
 
@@ -535,8 +544,9 @@ void Slice(const FunctionCallbackInfo<Value>& args) {
   Local<Uint8Array> ui = Uint8Array::New(ab, start, size);
   Maybe<bool> mb =
       ui->SetPrototype(env->context(), env->buffer_prototype_object());
-  if (mb.FromMaybe(false))
-    args.GetReturnValue().Set(ui);
+  if (!mb.FromMaybe(false))
+    env->ThrowError("Unable to set Object prototype");
+  args.GetReturnValue().Set(ui);
 }
 
 

src/node_buffer.h

@@ -24,50 +24,52 @@ NODE_EXTERN size_t Length(v8::Handle<v8::Value> val);
 NODE_EXTERN size_t Length(v8::Handle<v8::Object> val);
 
 // public constructor
-NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate, size_t length);
+NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate, size_t length);
 NODE_DEPRECATED("Use New(isolate, ...)",
-                inline v8::Local<v8::Object> New(size_t length) {
+                inline v8::MaybeLocal<v8::Object> New(size_t length) {
   return New(v8::Isolate::GetCurrent(), length);
 })
 // public constructor from string
-NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
-                                      v8::Handle<v8::String> string,
-                                      enum encoding enc = UTF8);
+NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
+                                           v8::Handle<v8::String> string,
+                                           enum encoding enc = UTF8);
 NODE_DEPRECATED("Use New(isolate, ...)",
-                inline v8::Local<v8::Object> New(v8::Handle<v8::String> string,
-                                                 enum encoding enc = UTF8) {
+                inline v8::MaybeLocal<v8::Object> New(
+                    v8::Handle<v8::String> string,
+                    enum encoding enc = UTF8) {
   return New(v8::Isolate::GetCurrent(), string, enc);
 })
 // public constructor - data is copied
 // TODO(trevnorris): should be something like Copy()
-NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
-                                      const char* data,
-                                      size_t len);
+NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
+                                           const char* data,
+                                           size_t len);
 NODE_DEPRECATED("Use New(isolate, ...)",
-                inline v8::Local<v8::Object> New(const char* data, size_t len) {
+                inline v8::MaybeLocal<v8::Object> New(const char* data,
+                                                      size_t len) {
   return New(v8::Isolate::GetCurrent(), data, len);
 })
 // public constructor - data is used, callback is passed data on object gc
-NODE_EXTERN v8::Local<v8::Object> New(v8::Isolate* isolate,
-                                      char* data,
-                                      size_t length,
-                                      FreeCallback callback,
-                                      void* hint);
+NODE_EXTERN v8::MaybeLocal<v8::Object> New(v8::Isolate* isolate,
+                                           char* data,
+                                           size_t length,
+                                           FreeCallback callback,
+                                           void* hint);
 NODE_DEPRECATED("Use New(isolate, ...)",
-                inline v8::Local<v8::Object> New(char* data,
-                                                 size_t length,
-                                                 FreeCallback callback,
-                                                 void* hint) {
+                inline v8::MaybeLocal<v8::Object> New(char* data,
+                                                      size_t length,
+                                                      FreeCallback callback,
+                                                      void* hint) {
   return New(v8::Isolate::GetCurrent(), data, length, callback, hint);
 })
 
 // public constructor - data is used.
 // TODO(trevnorris): should be New() for consistency
-NODE_EXTERN v8::Local<v8::Object> Use(v8::Isolate* isolate,
-                                      char* data,
-                                      size_t len);
+NODE_EXTERN v8::MaybeLocal<v8::Object> Use(v8::Isolate* isolate,
+                                           char* data,
+                                           size_t len);
 NODE_DEPRECATED("Use Use(isolate, ...)",
-                inline v8::Local<v8::Object> Use(char* data, size_t len) {
+                inline v8::MaybeLocal<v8::Object> Use(char* data, size_t len) {
   return Use(v8::Isolate::GetCurrent(), data, len);
 })
 
@@ -90,14 +92,14 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
 // src/node_internals.h due to a circular dependency issue with
 // the smalloc.h and node_internals.h headers.
 #if defined(NODE_WANT_INTERNALS)
-v8::Local<v8::Object> New(Environment* env, size_t size);
-v8::Local<v8::Object> New(Environment* env, const char* data, size_t len);
-v8::Local<v8::Object> New(Environment* env,
-                          char* data,
-                          size_t length,
-                          FreeCallback callback,
-                          void* hint);
-v8::Local<v8::Object> Use(Environment* env, char* data, size_t length);
+v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
+v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
+v8::MaybeLocal<v8::Object> New(Environment* env,
+                               char* data,
+                               size_t length,
+                               FreeCallback callback,
+                               void* hint);
+v8::MaybeLocal<v8::Object> Use(Environment* env, char* data, size_t length);
 #endif  // defined(NODE_WANT_INTERNALS)
 
 }  // namespace Buffer