Permalink
Browse files

domain: fix domain callback from MakeCallback

Since _tickCallback and _tickDomainCallback were both called from
MakeCallback, it was possible for a callback to be called that required
a domain directly to _tickCallback.

The fix was to implement process.usingDomains(). This will set all
applicable functions to their domain counterparts, and set a flag in cc
to let MakeCallback know domain callbacks always need to be checked.

Added test in own file. It's important that the test remains isolated.
  • Loading branch information...
1 parent a80a132 commit f0b68892d4e85c078836eb0809c64dde82918aeb @trevnorris trevnorris committed with isaacs Mar 26, 2013
Showing with 97 additions and 41 deletions.
  1. +2 −3 lib/domain.js
  2. +52 −34 src/node.cc
  3. +4 −4 src/node.js
  4. +39 −0 test/simple/test-domain-from-timer.js
View
@@ -32,9 +32,8 @@ var endMethods = ['end', 'abort', 'destroy', 'destroySoon'];
// a few side effects.
events.usingDomains = true;
-// replace tickers with domain specific implementation
-process.nextTick = process._nextDomainTick;
-process._tickCallback = process._tickDomainCallback;
+// let the process know we're using domains
+process._usingDomains();
exports.Domain = Domain;
View
@@ -102,7 +102,6 @@ Persistent<String> domain_symbol;
// declared in node_internals.h
Persistent<Object> process;
-static Persistent<Function> process_tickDomainCallback;
static Persistent<Function> process_tickFromSpinner;
static Persistent<Function> process_tickCallback;
@@ -134,6 +133,7 @@ static bool use_debug_agent = false;
static bool debug_wait_connect = false;
static int debug_port=5858;
static int max_stack_size = 0;
+static bool using_domains = false;
// used by C++ modules as well
bool no_deprecation = false;
@@ -899,24 +899,37 @@ Handle<Value> FromConstructorTemplate(Persistent<FunctionTemplate> t,
}
+Handle<Value> UsingDomains(const Arguments& args) {
+ HandleScope scope;
+ if (using_domains)
+ return scope.Close(Undefined());
+ using_domains = true;
+ Local<Value> tdc_v = process->Get(String::New("_tickDomainCallback"));
+ Local<Value> ndt_v = process->Get(String::New("_nextDomainTick"));
+ if (!tdc_v->IsFunction()) {
+ fprintf(stderr, "process._tickDomainCallback assigned to non-function\n");
+ abort();
+ }
+ if (!ndt_v->IsFunction()) {
+ fprintf(stderr, "process._nextDomainTick assigned to non-function\n");
+ abort();
+ }
+ Local<Function> tdc = tdc_v.As<Function>();
+ Local<Function> ndt = ndt_v.As<Function>();
+ process->Set(String::New("_tickCallback"), tdc);
+ process->Set(String::New("nextTick"), ndt);
+ process_tickCallback = Persistent<Function>::New(tdc);
+ return Undefined();
+}
+
+
Handle<Value>
MakeDomainCallback(const Handle<Object> object,
const Handle<Function> callback,
int argc,
Handle<Value> argv[]) {
// TODO Hook for long stack traces to be made here.
- // lazy load _tickDomainCallback
- if (process_tickDomainCallback.IsEmpty()) {
- Local<Value> cb_v = process->Get(String::New("_tickDomainCallback"));
- if (!cb_v->IsFunction()) {
- fprintf(stderr, "process._tickDomainCallback assigned to non-function\n");
- abort();
- }
- Local<Function> cb = cb_v.As<Function>();
- process_tickDomainCallback = Persistent<Function>::New(cb);
- }
-
// lazy load domain specific symbols
if (enter_symbol.IsEmpty()) {
enter_symbol = NODE_PSYMBOL("enter");
@@ -931,19 +944,22 @@ MakeDomainCallback(const Handle<Object> object,
TryCatch try_catch;
- domain = domain_v->ToObject();
- assert(!domain.IsEmpty());
- if (domain->Get(disposed_symbol)->IsTrue()) {
- // domain has been disposed of.
- return Undefined();
- }
- enter = Local<Function>::Cast(domain->Get(enter_symbol));
- assert(!enter.IsEmpty());
- enter->Call(domain, 0, NULL);
+ bool has_domain = domain_v->IsObject();
+ if (has_domain) {
+ domain = domain_v->ToObject();
+ assert(!domain.IsEmpty());
+ if (domain->Get(disposed_symbol)->IsTrue()) {
+ // domain has been disposed of.
+ return Undefined();
+ }
+ enter = Local<Function>::Cast(domain->Get(enter_symbol));
+ assert(!enter.IsEmpty());
+ enter->Call(domain, 0, NULL);
- if (try_catch.HasCaught()) {
- FatalException(try_catch);
- return Undefined();
+ if (try_catch.HasCaught()) {
+ FatalException(try_catch);
+ return Undefined();
+ }
}
Local<Value> ret = callback->Call(object, argc, argv);
@@ -953,13 +969,15 @@ MakeDomainCallback(const Handle<Object> object,
return Undefined();
}
- exit = Local<Function>::Cast(domain->Get(exit_symbol));
- assert(!exit.IsEmpty());
- exit->Call(domain, 0, NULL);
+ if (has_domain) {
+ exit = Local<Function>::Cast(domain->Get(exit_symbol));
+ assert(!exit.IsEmpty());
+ exit->Call(domain, 0, NULL);
- if (try_catch.HasCaught()) {
- FatalException(try_catch);
- return Undefined();
+ if (try_catch.HasCaught()) {
+ FatalException(try_catch);
+ return Undefined();
+ }
}
if (tick_infobox.length == 0) {
@@ -969,7 +987,7 @@ MakeDomainCallback(const Handle<Object> object,
}
// process nextTicks after call
- process_tickDomainCallback->Call(process, 0, NULL);
+ process_tickCallback->Call(process, 0, NULL);
if (try_catch.HasCaught()) {
FatalException(try_catch);
@@ -1033,10 +1051,8 @@ MakeCallback(const Handle<Object> object,
HandleScope scope;
Local<Function> callback = object->Get(symbol).As<Function>();
- Local<Value> domain = object->Get(domain_symbol);
- // has domain, off with you
- if (!domain->IsNull() && !domain->IsUndefined())
+ if (using_domains)
return scope.Close(MakeDomainCallback(object, callback, argc, argv));
return scope.Close(MakeCallback(object, callback, argc, argv));
}
@@ -2488,6 +2504,8 @@ Handle<Object> SetupProcessObject(int argc, char *argv[]) {
NODE_SET_METHOD(process, "binding", Binding);
+ NODE_SET_METHOD(process, "_usingDomains", UsingDomains);
+
// values use to cross communicate with processNextTick
Local<Object> info_box = Object::New();
info_box->SetIndexedPropertiesToExternalArrayData(&tick_infobox,
View
@@ -331,12 +331,12 @@
var index = 1;
var depth = 2;
- process._tickCallback = _tickCallback;
- process._tickFromSpinner = _tickFromSpinner;
- // needs to be accessible from cc land
- process._tickDomainCallback = _tickDomainCallback;
process.nextTick = nextTick;
+ // needs to be accessible from cc land
process._nextDomainTick = _nextDomainTick;
+ process._tickCallback = _tickCallback;
+ process._tickDomainCallback = _tickDomainCallback;
+ process._tickFromSpinner = _tickFromSpinner;
// the maximum number of times it'll process something like
// nextTick(function f(){nextTick(f)})
@@ -0,0 +1,39 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+// Simple tests of most basic domain functionality.
+
+var common = require('../common');
+var assert = require('assert');
+
+// timeouts call the callback directly from cc, so need to make sure the
+// domain will be used regardless
+setTimeout(function() {
+ var domain = require('domain');
+ var d = domain.create();
+ d.run(function() {
+ process.nextTick(function() {
+ console.trace('in nexttick', process.domain === d)
+ assert.equal(process.domain, d);
+ });
+ });
+});

0 comments on commit f0b6889

Please sign in to comment.