Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

callback may invoke twice #330

Closed
neutra opened this issue Sep 23, 2012 · 4 comments
Closed

callback may invoke twice #330

neutra opened this issue Sep 23, 2012 · 4 comments

Comments

@neutra
Copy link

neutra commented Sep 23, 2012

    // main.js
    117 |   // Protect against double callback
    118 |   if (!self._callback && self.callback) {
    119 |     self._callback = self.callback
    120 |     self.callback = function () {
    121 |       if (self._callbackCalled) return // Print a warning maybe?
    122 |       self._callback.apply(self, arguments)
    123 |       self._callbackCalled = true
    124 |     }
    125 |     self.on('error', self.callback.bind())
    126 |     self.on('complete', self.callback.bind(self, null))
    127 |   }

the flag '_callbackCalled ' will set at line 123, but the callback has called at line 122.
if the callback throw an exception, the callback might call twice. just like follow codes:

     var request, test;
     request = require('request');
     test = function() {
       var i, option;
       option = {
         uri: 'http://www.baidu.com',
         timeout: 5 * 1000
       };
       i = 1;
       return request(option, function(err) {
         console.dir([new Date(), i++]);
         throw new Error('oops');
       });
     };
     process.on('uncaughtException', function(err) {
       return console.log('Caught exception: ' + err.stack);
     });
     setTimeout(test, 200);
     setTimeout((function() {}), 100000);

how about swap line 122 and 123?

@mikeal
Copy link
Member

mikeal commented Sep 25, 2012

line 121 returns if _callbackCalled so it'll never hit line 122

@mikeal mikeal closed this as completed Sep 25, 2012
@neutra
Copy link
Author

neutra commented Sep 28, 2012

think about such situation:
an error occor while invoking the callback first time before timeout, line 123 will not execute, the flag '_callbackCalled ' still false.
assume that there is an ErrorHandler and the 'error' event has handled.
when timeout, line 121 will find that _callbackCalled still false and then invoke the callback again.

@nickminutello
Copy link

Excellent. I just hit this problem.
The other problem that can occur is a "RangeError: Maximum call stack size exceeded" if the callback emits an 'error' on the stream.

@mikeal mikeal reopened this Mar 12, 2013
@mikeal mikeal closed this as completed Mar 12, 2013
@mikeal
Copy link
Member

mikeal commented Mar 12, 2013

this should be fixed in master from the pull request i already accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants