othiym23 / node-continuation-local-storage

implementation of https://github.com/joyent/node/issues/5243
BSD 2-Clause "Simplified" License
1.13k stars 207 forks source link

TypeError: Cannot read property 'context@__NR_tracer' of undefined #12

Closed groundwater closed 10 years ago

groundwater commented 10 years ago

I'm narrowing in on the case of the undefined property. We've seen a few stack traces like this:

TypeError: Cannot read property 'context@__NR_tracer' of undefined
    at prepare (/Users/jacob/Projects/continuation-local-storage/context.js:123:32)
    at EventEmitter.punched [as emit] (/Users/jacob/Projects/continuation-local-storage/context.js:155:29)
    at Object.<anonymous> (/Users/jacob/Projects/continuation-local-storage/event.js:20:4)

Which I can reproduce as follow:

var events = require('events');
var cls    = require('continuation-local-storage');
var n      = cls.createNamespace('__NR_tracer');
var ee     = new events.EventEmitter();

function H1() {
  console.log(1);
}

n.bindEmitter(ee);

ee.on('test', H1);
ee.on('test', H1);

var x;
ee._events['test'][1] = x;
ee.emit('test');

I have to directly fiddle with the event emitters events to reproduce the error, but it's definitely raising the right error.

  1. we should find out what modules / natural causes this situation can arise
  2. we should make CLS robust against shitty event emitters

Also I noticed in node 0.8.25 there is no error thrown, the app just silently exits.

groundwater commented 10 years ago

Note that if I disable CLS entirely, the app will still error with

TypeError: Cannot call method 'apply' of undefined
    at EventEmitter.emit (events.js:117:20)
    at Object.<anonymous> (/Users/jacob/Projects/continuation-local-storage/event.js:17:4)

It's possible CLS is just getting the blame for an inevitable crash, but perhaps CLS is actually causing the null listener in the first place.

othiym23 commented 10 years ago

I have a fix for this, I just need to write a test. To reproduce, you need a single EE listener for an event that isn't bound to a CLS context. If you look at the logic in puncher you'll see why -- I have a test that's using thing.length where it should be using Array.isArray instead.

groundwater commented 10 years ago

If unwrapped is a function, then unwrapped.length==0 and it skips that block of code. Do you have an example where unwrapped has positive length, but is a function?

othiym23 commented 10 years ago

.length on a function is its arity. ;)

othiym23 commented 10 years ago

(That was exactly the logic I used when I was writing that code, and it worked for so long because listeners that take parameters are pretty rare.)

groundwater commented 10 years ago
var events = require('events');
var cls    = require('continuation-local-storage');
var n      = cls.createNamespace('__NR_tracer');
var ee     = new events.EventEmitter();

function H1(a) {
  console.log(1);
}

ee.on('test', H1);
n.bindEmitter(ee);
ee.emit('test');

BOOM, HEADSHOT

TypeError: Cannot read property 'context@__NR_tracer' of undefined
othiym23 commented 10 years ago

Correclty! WTG me.