lib,src: replace all C++ promises with JS promises

BridgeAR · targos · commit 9817e405eefd · 2018-07-24T20:30:24.000+02:00
C++ promises can not be properly optimized by V8. They also behave a tiny bit different than "regular" promises. PR-URL: #20830 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Backport-PR-URL: #21879 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git a/lib/child_process.js b/lib/child_process.js
@@ -26,8 +26,6 @@ const {
 deprecate, convertToValidSignal, getSystemErrorName
 } = require('internal/util');
 const { isUint8Array } = require('internal/util/types');
-const { createPromise,
- promiseResolve, promiseReject } = process.binding('util');
 const debug = util.debuglog('child_process');
 const { Buffer } = require('buffer');
 const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
@@ -152,18 +150,17 @@ exports.exec = function exec(command /* , options, callback */) {
 
 const customPromiseExecFunction = (orig) => {
 return (...args) => {
- const promise = createPromise();
-
- orig(...args, (err, stdout, stderr) => {
- if (err !== null) {
- err.stdout = stdout;
- err.stderr = stderr;
- promiseReject(promise, err);
- } else {
- promiseResolve(promise, { stdout, stderr });
- }
+ return new Promise((resolve, reject) => {
+ orig(...args, (err, stdout, stderr) => {
+ if (err !== null) {
+ err.stdout = stdout;
+ err.stderr = stderr;
+ reject(err);
+ } else {
+ resolve({ stdout, stderr });
+ }
+ });
 });
- return promise;
 };
 };
 
diff --git a/lib/fs.js b/lib/fs.js
@@ -207,10 +207,7 @@ function exists(path, callback) {
 
 Object.defineProperty(exists, internalUtil.promisify.custom, {
 value: (path) => {
- const { createPromise, promiseResolve } = process.binding('util');
- const promise = createPromise();
- fs.exists(path, (exists) => promiseResolve(promise, exists));
- return promise;
+ return new Promise((resolve) => fs.exists(path, resolve));
 }
 });
 
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -113,7 +113,6 @@ const { isArrayBufferView } = require('internal/util/types');
 const { FileHandle } = process.binding('fs');
 const binding = process.binding('http2');
 const { ShutdownWrap } = process.binding('stream_wrap');
-const { createPromise, promiseResolve } = process.binding('util');
 const { UV_EOF } = process.binding('uv');
 
 const { StreamPipe } = internalBinding('stream_pipe');
@@ -2747,11 +2746,9 @@ function connect(authority, options, listener) {
 // Support util.promisify
 Object.defineProperty(connect, promisify.custom, {
 value: (authority, options) => {
- const promise = createPromise();
- const server = connect(authority,
- options,
- () => promiseResolve(promise, server));
- return promise;
+ return new Promise((resolve) => {
+ const server = connect(authority, options, () => resolve(server));
+ });
 }
 });
 
diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -8,10 +8,7 @@ const {
 const { signals } = process.binding('constants').os;
 
 const {
- createPromise,
 getHiddenValue,
- promiseResolve,
- promiseReject,
 setHiddenValue,
 arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
 decorated_private_symbol: kDecoratedPrivateSymbolIndex
@@ -276,24 +273,21 @@ function promisify(original) {
 const argumentNames = original[kCustomPromisifyArgsSymbol];
 
 function fn(...args) {
- const promise = createPromise();
- try {
+ return new Promise((resolve, reject) => {
 original.call(this, ...args, (err, ...values) => {
 if (err) {
- promiseReject(promise, err);
- } else if (argumentNames !== undefined && values.length > 1) {
+ return reject(err);
+ }
+ if (argumentNames !== undefined && values.length > 1) {
 const obj = {};
 for (var i = 0; i < argumentNames.length; i++)
 obj[argumentNames[i]] = values[i];
- promiseResolve(promise, obj);
+ resolve(obj);
 } else {
- promiseResolve(promise, values[0]);
+ resolve(values[0]);
 }
 });
- } catch (err) {
- promiseReject(promise, err);
- }
- return promise;
+ });
 }
 
 Object.setPrototypeOf(fn, Object.getPrototypeOf(original));
diff --git a/lib/timers.js b/lib/timers.js
@@ -34,7 +34,6 @@ const {
 validateTimerDuration
 } = require('internal/timers');
 const internalUtil = require('internal/util');
-const { createPromise, promiseResolve } = process.binding('util');
 const assert = require('assert');
 const util = require('util');
 const { ERR_INVALID_CALLBACK } = require('internal/errors').codes;
@@ -407,11 +406,9 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
 }
 
 setTimeout[internalUtil.promisify.custom] = function(after, value) {
- const promise = createPromise();
- const timeout = new Timeout(promise, after, [value], false, false);
- active(timeout);
-
- return promise;
+ return new Promise((resolve) => {
+ active(new Timeout(resolve, after, [value], false, false));
+ });
 };
 
 exports.setTimeout = setTimeout;
@@ -420,7 +417,7 @@ exports.setTimeout = setTimeout;
 function ontimeout(timer, start) {
 const args = timer._timerArgs;
 if (typeof timer._onTimeout !== 'function')
- return promiseResolve(timer._onTimeout, args[0]);
+ return Promise.resolve(timer._onTimeout, args[0]);
 if (start === undefined && timer._repeat)
 start = TimerWrap.now();
 if (!args)
@@ -691,7 +688,7 @@ function tryOnImmediate(immediate, oldTail, count, refCount) {
 function runCallback(timer) {
 const argv = timer._argv;
 if (typeof timer._onImmediate !== 'function')
- return promiseResolve(timer._onImmediate, argv[0]);
+ return Promise.resolve(timer._onImmediate, argv[0]);
 if (!argv)
 return timer._onImmediate();
 Reflect.apply(timer._onImmediate, timer, argv);
@@ -766,9 +763,7 @@ function setImmediate(callback, arg1, arg2, arg3) {
 }
 
 setImmediate[internalUtil.promisify.custom] = function(value) {
- const promise = createPromise();
- new Immediate(promise, [value]);
- return promise;
+ return new Promise((resolve) => new Immediate(resolve, [value]));
 };
 
 exports.setImmediate = setImmediate;
diff --git a/src/node_util.cc b/src/node_util.cc
@@ -9,7 +9,6 @@ using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Integer;
 using v8::Local;
-using v8::Maybe;
 using v8::Object;
 using v8::Private;
 using v8::Promise;
@@ -127,36 +126,6 @@ void WatchdogHasPendingSigint(const FunctionCallbackInfo<Value>& args) {
 args.GetReturnValue().Set(ret);
 }
 
-
-void CreatePromise(const FunctionCallbackInfo<Value>& args) {
- Local<Context> context = args.GetIsolate()->GetCurrentContext();
- auto maybe_resolver = Promise::Resolver::New(context);
- if (!maybe_resolver.IsEmpty())
- args.GetReturnValue().Set(maybe_resolver.ToLocalChecked());
-}
-
-
-void PromiseResolve(const FunctionCallbackInfo<Value>& args) {
- Local<Context> context = args.GetIsolate()->GetCurrentContext();
- Local<Value> promise = args[0];
- CHECK(promise->IsPromise());
- if (promise.As<Promise>()->State() != Promise::kPending) return;
- Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
- Maybe<bool> ret = resolver->Resolve(context, args[1]);
- args.GetReturnValue().Set(ret.FromMaybe(false));
-}
-
-
-void PromiseReject(const FunctionCallbackInfo<Value>& args) {
- Local<Context> context = args.GetIsolate()->GetCurrentContext();
- Local<Value> promise = args[0];
- CHECK(promise->IsPromise());
- if (promise.As<Promise>()->State() != Promise::kPending) return;
- Local<Promise::Resolver> resolver = promise.As<Promise::Resolver>(); // sic
- Maybe<bool> ret = resolver->Reject(context, args[1]);
- args.GetReturnValue().Set(ret.FromMaybe(false));
-}
-
 void SafeGetenv(const FunctionCallbackInfo<Value>& args) {
 CHECK(args[0]->IsString());
 Utf8Value strenvtag(args.GetIsolate(), args[0]);
@@ -211,10 +180,6 @@ void Initialize(Local<Object> target,
 env->SetMethodNoSideEffect(target, "watchdogHasPendingSigint",
 WatchdogHasPendingSigint);
 
- env->SetMethodNoSideEffect(target, "createPromise", CreatePromise);
- env->SetMethod(target, "promiseResolve", PromiseResolve);
- env->SetMethod(target, "promiseReject", PromiseReject);
-
 env->SetMethod(target, "safeGetenv", SafeGetenv);
 }
 
diff --git a/test/parallel/test-promise-internal-creation.js b/test/parallel/test-promise-internal-creation.js

-Original file line number
+Diff line change
  deprecate, convertToValidSignal, getSystemErrorName
 }=require('internal/util');
 const{ isUint8Array }=require('internal/util/types');
 -const{ createPromise,
 - promiseResolve, promiseReject }=process.binding('util');
 constdebug=util.debuglog('child_process');
 const{ Buffer }=require('buffer');
 const{ Pipe,constants: PipeConstants}=process.binding('pipe_wrap');
 constcustomPromiseExecFunction=(orig)=>{
 return(...args)=>{
 -constpromise=createPromise();
+-
 -orig(...args,(err,stdout,stderr)=>{
 -if(err!==null){
 -err.stdout=stdout;
 -err.stderr=stderr;
 -promiseReject(promise,err);
 -}else{
 -promiseResolve(promise,{ stdout, stderr });
 -}
 +returnnewPromise((resolve,reject)=>{
 +orig(...args,(err,stdout,stderr)=>{
 +if(err!==null){
 +err.stdout=stdout;
 +err.stderr=stderr;
 +reject(err);
 +}else{
 +resolve({ stdout, stderr });
 +}
 +});
 });
 -returnpromise;
 };
 };
-Original file line number
+Diff line change
 Object.defineProperty(exists,internalUtil.promisify.custom,{
 value: (path)=>{
 -const{ createPromise, promiseResolve }=process.binding('util');
 -constpromise=createPromise();
 -fs.exists(path,(exists)=>promiseResolve(promise,exists));
 -returnpromise;
 +returnnewPromise((resolve)=>fs.exists(path,resolve));
+}
 });
-Original file line number
+Diff line change
 const{ FileHandle }=process.binding('fs');
 constbinding=process.binding('http2');
 const{ ShutdownWrap }=process.binding('stream_wrap');
 -const{ createPromise, promiseResolve }=process.binding('util');
 const{UV_EOF}=process.binding('uv');
 const{ StreamPipe }=internalBinding('stream_pipe');
 // Support util.promisify
 Object.defineProperty(connect,promisify.custom,{
 value: (authority,options)=>{
 -constpromise=createPromise();
 -constserver=connect(authority,
 -options,
 -()=>promiseResolve(promise,server));
 -returnpromise;
 +returnnewPromise((resolve)=>{
 +constserver=connect(authority,options,()=>resolve(server));
 +});
+}
 });
-Original file line number
+Diff line change
  validateTimerDuration
 }=require('internal/timers');
 constinternalUtil=require('internal/util');
 -const{ createPromise, promiseResolve }=process.binding('util');
 constassert=require('assert');
 constutil=require('util');
 const{ERR_INVALID_CALLBACK}=require('internal/errors').codes;
+}
 setTimeout[internalUtil.promisify.custom]=function(after,value){
 -constpromise=createPromise();
 -consttimeout=newTimeout(promise,after,[value],false,false);
 -active(timeout);
+-
 -returnpromise;
 +returnnewPromise((resolve)=>{
 +active(newTimeout(resolve,after,[value],false,false));
 +});
 };
 exports.setTimeout=setTimeout;
 functionontimeout(timer,start){
 constargs=timer._timerArgs;
 if(typeoftimer._onTimeout!=='function')
 -returnpromiseResolve(timer._onTimeout,args[0]);
 +returnPromise.resolve(timer._onTimeout,args[0]);
 if(start===undefined&&timer._repeat)
 start=TimerWrap.now();
 if(!args)
 functionrunCallback(timer){
 constargv=timer._argv;
 if(typeoftimer._onImmediate!=='function')
 -returnpromiseResolve(timer._onImmediate,argv[0]);
 +returnPromise.resolve(timer._onImmediate,argv[0]);
 if(!argv)
 returntimer._onImmediate();
 Reflect.apply(timer._onImmediate,timer,argv);
+}
 setImmediate[internalUtil.promisify.custom]=function(value){
 -constpromise=createPromise();
 -newImmediate(promise,[value]);
 -returnpromise;
 +returnnewPromise((resolve)=>newImmediate(resolve,[value]));
 };
 exports.setImmediate=setImmediate;