Skip to content

Commit 86c0745

Browse files
trevnorrisisaacs
trevnorris
authored and
isaacs
committed
process: streamlining tick callback logic
* Callbacks from spinner now calls its own function, separate from the tickCallback logic * MakeCallback will call a domain specific function if a domain is detected * _tickCallback assumes no domains, until nextTick receives a callback with a domain. After that _tickCallback is overridden with the domain specific implementation. * _needTickCallback runs in startup() instead of nextTick (isaacs) * Fix bug in _fatalException where exit would be called twice (isaacs) * Process.domain has a default value of null * Manually track nextTickQueue.length (will be useful later) * Update tests to reflect internal api changes
1 parent 95ac576 commit 86c0745

File tree

7 files changed

+234
-112
lines changed

7 files changed

+234
-112
lines changed

‎src/node.cc‎

Lines changed: 99 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ Persistent<String> process_symbol;
100100
Persistent<String> domain_symbol;
101101

102102
static Persistent<Object> process;
103+
static Persistent<Function> process_tickWDCallback;
104+
static Persistent<Function> process_tickFromSpinner;
103105
static Persistent<Function> process_tickCallback;
104106

105107
static Persistent<String> exports_symbol;
@@ -174,20 +176,19 @@ static void Tick(void) {
174176

175177
HandleScope scope;
176178

177-
if (tick_callback_sym.IsEmpty()) {
178-
// Lazily set the symbol
179-
tick_callback_sym = NODE_PSYMBOL("_tickCallback");
179+
if (process_tickFromSpinner.IsEmpty()) {
180+
Local<Value> cb_v = process->Get(String::New("_tickFromSpinner"));
181+
if (!cb_v->IsFunction()) {
182+
fprintf(stderr, "process._tickFromSpinner assigned to non-function\n");
183+
abort();
184+
}
185+
Local<Function> cb = cb_v.As<Function>();
186+
process_tickFromSpinner = Persistent<Function>::New(cb);
180187
}
181188

182-
Local<Value> cb_v = process->Get(tick_callback_sym);
183-
if (!cb_v->IsFunction()) return;
184-
Local<Function> cb = Local<Function>::Cast(cb_v);
185-
186189
TryCatch try_catch;
187190

188-
// Let the tick callback know that this is coming from the spinner
189-
Handle<Value> argv[] = { True(node_isolate) };
190-
cb->Call(process, ARRAY_SIZE(argv), argv);
191+
process_tickFromSpinner->Call(process, 0, NULL);
191192

192193
if (try_catch.HasCaught()) {
193194
FatalException(try_catch);
@@ -903,46 +904,23 @@ Handle<Value> FromConstructorTemplate(Persistent<FunctionTemplate> t,
903904
}
904905

905906

906-
// MakeCallback may only be made directly off the event loop.
907-
// That is there can be no JavaScript stack frames underneath it.
908-
// (Is there any way to assert that?)
909-
//
910-
// Maybe make this a method of a node::Handle super class
911-
//
912-
Handle<Value>
913-
MakeCallback(const Handle<Object> object,
914-
const char* method,
915-
int argc,
916-
Handle<Value> argv[]) {
917-
HandleScope scope;
918-
919-
Handle<Value> ret =
920-
MakeCallback(object, String::NewSymbol(method), argc, argv);
921-
922-
return scope.Close(ret);
923-
}
924-
925907
Handle<Value>
926-
MakeCallback(const Handle<Object> object,
927-
const Handle<String> symbol,
908+
MCWithDomain(const Handle<Object> object,
909+
const Handle<Function> callback,
928910
int argc,
929911
Handle<Value> argv[]) {
930-
HandleScope scope;
931-
932-
Local<Value> callback_v = object->Get(symbol);
933-
if (!callback_v->IsFunction()) {
934-
String::Utf8Value method(symbol);
935-
// XXX: If the object has a domain attached, handle it there?
936-
// At least, would be good to get *some* sort of indication
937-
// of how we got here, even if it's not catchable.
938-
fprintf(stderr, "Non-function in MakeCallback. method = %s\n", *method);
939-
abort();
912+
// lazy load _tickWDCallback
913+
if (process_tickWDCallback.IsEmpty()) {
914+
Local<Value> cb_v = process->Get(String::New("_tickWDCallback"));
915+
if (!cb_v->IsFunction()) {
916+
fprintf(stderr, "process._tickWDCallback assigned to non-function\n");
917+
abort();
918+
}
919+
Local<Function> cb = cb_v.As<Function>();
920+
process_tickWDCallback = Persistent<Function>::New(cb);
940921
}
941922

942-
// TODO Hook for long stack traces to be made here.
943-
944-
TryCatch try_catch;
945-
923+
// lazy load domain specific symbols
946924
if (enter_symbol.IsEmpty()) {
947925
enter_symbol = NODE_PSYMBOL("enter");
948926
exit_symbol = NODE_PSYMBOL("exit");
@@ -953,40 +931,71 @@ MakeCallback(const Handle<Object> object,
953931
Local<Object> domain;
954932
Local<Function> enter;
955933
Local<Function> exit;
956-
if (!domain_v->IsUndefined()) {
957-
domain = domain_v->ToObject();
958-
if (domain->Get(disposed_symbol)->BooleanValue()) {
959-
// domain has been disposed of.
960-
return Undefined(node_isolate);
961-
}
962-
enter = Local<Function>::Cast(domain->Get(enter_symbol));
963-
enter->Call(domain, 0, NULL);
934+
935+
TryCatch try_catch;
936+
937+
domain = domain_v->ToObject();
938+
assert(!domain.IsEmpty());
939+
if (domain->Get(disposed_symbol)->IsTrue()) {
940+
// domain has been disposed of.
941+
return Undefined(node_isolate);
964942
}
943+
enter = Local<Function>::Cast(domain->Get(enter_symbol));
944+
assert(!enter.IsEmpty());
945+
enter->Call(domain, 0, NULL);
965946

966947
if (try_catch.HasCaught()) {
967948
FatalException(try_catch);
968949
return Undefined(node_isolate);
969950
}
970951

971-
Local<Function> callback = Local<Function>::Cast(callback_v);
972952
Local<Value> ret = callback->Call(object, argc, argv);
973953

974954
if (try_catch.HasCaught()) {
975955
FatalException(try_catch);
976956
return Undefined(node_isolate);
977957
}
978958

979-
if (!domain_v->IsUndefined()) {
980-
exit = Local<Function>::Cast(domain->Get(exit_symbol));
981-
exit->Call(domain, 0, NULL);
959+
exit = Local<Function>::Cast(domain->Get(exit_symbol));
960+
assert(!exit.IsEmpty());
961+
exit->Call(domain, 0, NULL);
962+
963+
if (try_catch.HasCaught()) {
964+
FatalException(try_catch);
965+
return Undefined(node_isolate);
982966
}
983967

968+
// process nextTicks after call
969+
process_tickWDCallback->Call(process, 0, NULL);
970+
984971
if (try_catch.HasCaught()) {
985972
FatalException(try_catch);
986973
return Undefined(node_isolate);
987974
}
988975

989-
// process nextTicks after every time we get called.
976+
return ret;
977+
}
978+
979+
980+
Handle<Value>
981+
MakeCallback(const Handle<Object> object,
982+
const Handle<String> symbol,
983+
int argc,
984+
Handle<Value> argv[]) {
985+
HandleScope scope;
986+
987+
Local<Value> callback_v = object->Get(symbol);
988+
if (!callback_v->IsFunction()) {
989+
String::Utf8Value method(symbol);
990+
fprintf(stderr, "Non-function in MakeCallback. method = %s\n", *method);
991+
abort();
992+
}
993+
994+
Local<Function> callback = Local<Function>::Cast(callback_v);
995+
996+
// TODO Hook for long stack traces to be made here.
997+
998+
// lazy load no domain next tick callbacks
990999
if (process_tickCallback.IsEmpty()) {
9911000
Local<Value> cb_v = process->Get(String::New("_tickCallback"));
9921001
if (!cb_v->IsFunction()) {
@@ -996,7 +1005,24 @@ MakeCallback(const Handle<Object> object,
9961005
Local<Function> cb = cb_v.As<Function>();
9971006
process_tickCallback = Persistent<Function>::New(cb);
9981007
}
999-
process_tickCallback->Call(process, NULL, 0);
1008+
1009+
// has domain, off with you
1010+
if (object->Has(domain_symbol) &&
1011+
!object->Get(domain_symbol)->IsNull() &&
1012+
!object->Get(domain_symbol)->IsUndefined())
1013+
return scope.Close(MCWithDomain(object, callback, argc, argv));
1014+
1015+
TryCatch try_catch;
1016+
1017+
Local<Value> ret = callback->Call(object, argc, argv);
1018+
1019+
if (try_catch.HasCaught()) {
1020+
FatalException(try_catch);
1021+
return Undefined(node_isolate);
1022+
}
1023+
1024+
// process nextTicks after call
1025+
process_tickCallback->Call(process, 0, NULL);
10001026

10011027
if (try_catch.HasCaught()) {
10021028
FatalException(try_catch);
@@ -1007,6 +1033,20 @@ MakeCallback(const Handle<Object> object,
10071033
}
10081034

10091035

1036+
Handle<Value>
1037+
MakeCallback(const Handle<Object> object,
1038+
const char* method,
1039+
int argc,
1040+
Handle<Value> argv[]) {
1041+
HandleScope scope;
1042+
1043+
Handle<Value> ret =
1044+
MakeCallback(object, String::NewSymbol(method), argc, argv);
1045+
1046+
return scope.Close(ret);
1047+
}
1048+
1049+
10101050
void SetErrno(uv_err_t err) {
10111051
HandleScope scope;
10121052

0 commit comments

Comments
 (0)