Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

events: Don't assume .domain is a Domain instance #3998

Closed
wants to merge 1 commit into from

4 participants

@bentaber

EventEmitter assumes that anything with a .domain property is an instance of a
Domain.Domain. This clobbers any EventEmitter instances/inheriters that make
use of the fairly generalized property name. Change is backwards compatible
with existing interface. New test included.

Fixes #3922

I know pull request 3932 is already open, but this would allow this to be fixed
in the next v0.8 release as opposed to 1.0.

#3932

@bentaber bentaber events: Don't assume .domain is a Domain instance
EventEmitter assumes that anything with a .domain property is an instance of a
Domain.Domain.  This clobbers any EventEmitter instances/inheriters that make
use of the fairly generalized property name.  Change is backwards compatible
with existing interface.  New test included.

Fixes #3922
9d4b1c2
@Nodejs-Jenkins

Can one of the admins verify this patch?

@isaacs
Owner

This has been resolved by other issues, and besides, does not merge cleanly. Re-submit if it's still a thing.

@isaacs isaacs closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 11, 2012
  1. @bentaber

    events: Don't assume .domain is a Domain instance

    bentaber authored
    EventEmitter assumes that anything with a .domain property is an instance of a
    Domain.Domain.  This clobbers any EventEmitter instances/inheriters that make
    use of the fairly generalized property name.  Change is backwards compatible
    with existing interface.  New test included.
    
    Fixes #3922
This page is out of date. Refresh to see the latest.
Showing with 61 additions and 5 deletions.
  1. +9 −5 lib/events.js
  2. +52 −0 test/simple/test-domain-clobber-property.js
View
14 lib/events.js
@@ -22,6 +22,10 @@
var isArray = Array.isArray;
var domain;
+function isDomain(ee) {
+ return domain && ee.domain && ee.domain instanceof domain.Domain;
+}
+
function EventEmitter() {
if (exports.usingDomains) {
// if there is an active domain, then attach to it.
@@ -53,7 +57,7 @@ EventEmitter.prototype.emit = function() {
if (!this._events || !this._events.error ||
(isArray(this._events.error) && !this._events.error.length))
{
- if (this.domain) {
+ if (isDomain(this)) {
var er = arguments[1];
er.domain_emitter = this;
er.domain = this.domain;
@@ -76,7 +80,7 @@ EventEmitter.prototype.emit = function() {
if (!handler) return false;
if (typeof handler == 'function') {
- if (this.domain) {
+ if (isDomain(this)) {
this.domain.enter();
}
switch (arguments.length) {
@@ -97,13 +101,13 @@ EventEmitter.prototype.emit = function() {
for (var i = 1; i < l; i++) args[i - 1] = arguments[i];
handler.apply(this, args);
}
- if (this.domain) {
+ if (isDomain(this)) {
this.domain.exit();
}
return true;
} else if (isArray(handler)) {
- if (this.domain) {
+ if (isDomain(this)) {
this.domain.enter();
}
var l = arguments.length;
@@ -114,7 +118,7 @@ EventEmitter.prototype.emit = function() {
for (var i = 0, l = listeners.length; i < l; i++) {
listeners[i].apply(this, args);
}
- if (this.domain) {
+ if (isDomain(this)) {
this.domain.exit();
}
return true;
View
52 test/simple/test-domain-clobber-property.js
@@ -0,0 +1,52 @@
+// 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.
+
+// Make sure using domains doesn't clobber eventemitters with .domain properties
+// https://github.com/joyent/node/issues/3922
+
+var assert = require('assert');
+var events = require('events');
+var util = require('util');
+
+var Emitter = function() {
+ this.domain = 'abc.com';
+};
+util.inherits(Emitter, events.EventEmitter);
+
+
+function test(callback) {
+ var emitter = new Emitter();
+
+ emitter.on('test', function() {
+ assert.equal('abc.com', emitter.domain);
+ if (callback) callback();
+ });
+
+ assert.doesNotThrow(function() {
+ emitter.emit('test');
+ });
+}
+
+test(function() {
+ // require ('domain') and try it again
+ var domain = require('domain');
+ test();
+});
Something went wrong with that request. Please try again.