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

possible EventEmitter memory leak warning #2122

Closed
vincentchu opened this issue Sep 18, 2017 · 21 comments
Closed

possible EventEmitter memory leak warning #2122

vincentchu opened this issue Sep 18, 2017 · 21 comments

Comments

@vincentchu
Copy link

vincentchu commented Sep 18, 2017

  • Was developing all weekend with MetaMask and I can swear that this warning never popped up until just today:

  • contentscript.js:2532 (node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.

  • When seeing this issue, events no longer were syncing between my local testnet and Metamask

  • Wonder if this had anything with the new release that went out 9/18/2017?

  • I searched in the current issues and only old issues were found but I wonder if they're related somehow?

Thanks ...

@2-am-zzz
Copy link
Contributor

We've had this warning appear for awhile, so it's nothing new. @kumavis was any of your filter work related to listeners?

@danfinlay
Copy link
Contributor

Probably worth asking: what console are you seeing this warning in? If it's the web page console, that might actually be new.

@adklempner
Copy link

I am also getting this error through Google Chrome console. It's only occurred after updating to 3.10.1

It occurs on every page, before MetaMask is injected it seems.
screen shot 2017-09-18 at 4 05 42 pm

@danfinlay
Copy link
Contributor

We are managing event subscriptions differently now, so it's very possible these new warnings are not actual problems, but we'll revisit them, because we should stay quiet if nothing is wrong, especially in the Ðapp's console.

@adklempner
Copy link

web3.eth.accounts[0] stops updating after the error occurs. So if MetaMask is locked when the page loads, and is then unlocked, web3.eth.accounts[0] still returns undefined

@danfinlay
Copy link
Contributor

Well that makes it sound much more serious. I just left my computer for a few hours, but we'll be looking at this asap.

@vincentchu
Copy link
Author

@FlySwatter Thanks for the quick response--- very appreciated. I'm seeing this in the chrome console. FWIW, after this change, I'm no longer able to read events off of my locally deployed smart contracts.

@VogtAI
Copy link

VogtAI commented Sep 19, 2017

For me it's a breaking change. My DApp no longer handles event logs properly without any changes to my code.

@vincentchu
Copy link
Author

@VogtAI What were the changes you had to make?

@VogtAI
Copy link

VogtAI commented Sep 19, 2017

@vincentchu I phrased it poorly. I meant to say that I didn't change anything in my code, so I'm sure the MetaMask update broke my DApp.

@coopermaruyama
Copy link

coopermaruyama commented Sep 19, 2017

Might be related - My MetaMask stopped working all of the sudden today. It crashes right away when I click on the icon. I can confirm that after downgrading to 3.10.0, MetaMask works again. This happens for me only on google chrome canary. works fine in regular chrome browser. Looking at the error logs I see a bunch of the EventEmitter warnings you described.

@vsergeev
Copy link

vsergeev commented Sep 19, 2017

@vincentchu, @VogtAI, ❗ same thing happened to me. Event fetching is now broken in my DApp.

Edit: my setup is running with testrpc on http://localhost:8545, event fetch is of the form contractInstance.myEvent(null, {fromBlock: 0, toBlock: 'latest'}, callback). MetaMask version 3.10.0 works fine.

@danfinlay
Copy link
Contributor

We've re-published 3.10.0 as 3.10.2, so your users should have the hot fix soon.

@danfinlay
Copy link
Contributor

We'll be looking closely at the changes and fixing the issue before republishing.

@danfinlay
Copy link
Contributor

We believe we have fixed this issue, could affected developers test this custom build against your dapps, and confirm this fixes this problem?

metamask-chrome-3.10.3-pre.zip

@adklempner
Copy link

3.10.3-pre gives me the same issue

BTW, had to remove -pre from the version in manifest.json to manually install it in Chrome

@danfinlay
Copy link
Contributor

@adklempner Could you provide us with reproduction steps? We fixed the issue we were able to reproduce, now we're not so sure.

@danfinlay
Copy link
Contributor

Updated version that fixes the problem @adklempner noted:

metamask-chrome-3.10.3-pre2.zip

@vsergeev
Copy link

vsergeev commented Sep 20, 2017

Thanks for the quick turn around. Unfortunately, no luck with 3.10.3-pre2 for me.

Test.sol

pragma solidity ^0.4.4;

contract Test {
    event Foobar(uint256 x);

    function Test() {
        Foobar(1);
        Foobar(2);
        Foobar(3);
    }
}

app.js

window.onload = function () {
  if (typeof web3 !== 'undefined') {
    web3 = new Web3(web3.currentProvider);
  }

  var abi = [{ "inputs": [], "payable": false, "type": "constructor" }, { "anonymous": false, "inputs": [ { "indexed": false, "name": "x", "type": "uint256" } ], "name": "Foobar", "type": "event" } ];
  var address = "0x42fdca8935689fcdf2baedc707de6da978515564";
  var TestContract = web3.eth.contract(abi);
  var testInstance = TestContract.at(address);

  var foobarEvent = testInstance.Foobar(null, {fromBlock: 0, toBlock: 'latest'}, function (error, result) {
    if (error)
      console.error(error)
    else
      console.log("Got a foobar with " + result.args.x.toString());
  });
}

index.html

<!DOCTYPE html>
<html lang="en">
  <head></head>
  <body>
  <script src="web3.min.js"></script>
  <script src="app.js"></script>
  </body>
</html>

web3.version.api: "0.20.1"

Result (expected) with 3.10.0:

MetaMask - injected web3
Got a foobar with 1
Got a foobar with 2
Got a foobar with 3

result1

Result with 3.10.3-pre2:

<event emitter warnings>
MetaMask - injected web3

result2

Chromium version: Version 60.0.3112.113 (Developer Build) (64-bit)

testrpc version: EthereumJS TestRPC v4.1.1 (ganache-core: 1.1.2)

@e00dan
Copy link

e00dan commented Jan 24, 2019

EDIT

FOUND SOLUTION TO MY PROBLEM HERE:

#5493 (comment)

Original comment

I'm not sure if it's been fixed. I get the same error, just the number changed from 11 to 101. I think working with 101 contracts at once in dapp is not something unusual.

Error: Possible EventEmitter memory leak detected. 101 data listeners added. Use emitter.setMaxListeners() to increase limit

It's coming from Contract constructor, then packageInit and source seems to be setProvider:

 if(this.provider && this.provider.on) {
        this.provider.on('data', function requestManagerNotification(result, deprecatedResult){
            result = result || deprecatedResult; // this is for possible old providers, which may had the error first handler

            // check for result.method, to prevent old providers errors to pass as result
            if(result.method && _this.subscriptions[result.params.subscription] && _this.subscriptions[result.params.subscription].callback) {
                _this.subscriptions[result.params.subscription].callback(null, result.params.result);
            }
        });
        // TODO add error, end, timeout, connect??
        // this.provider.on('error', function requestManagerNotification(result){
        //     Object.keys(_this.subscriptions).forEach(function(id){
        //         if(_this.subscriptions[id].callback)
        //             _this.subscriptions[id].callback(err);
        //     });
        // }
    }

that is called from packageInit:

pkg._requestManager.setProvider(args[0], args[1]);

that is called from Contract constructor:

 // sets _requestmanager
    core.packageInit(this, [this.constructor.currentProvider]);

This means setProvider is called for each contract constructed? If yes, then it means it's not possible to call new Contract on one page more than 101 times?

@W3stside
Copy link

EDIT

FOUND SOLUTION TO MY PROBLEM HERE:

#5493 (comment)

Original comment

I'm not sure if it's been fixed. I get the same error, just the number changed from 11 to 101. I think working with 101 contracts at once in dapp is not something unusual.

Error: Possible EventEmitter memory leak detected. 101 data listeners added. Use emitter.setMaxListeners() to increase limit

It's coming from Contract constructor, then packageInit and source seems to be setProvider:

 if(this.provider && this.provider.on) {
        this.provider.on('data', function requestManagerNotification(result, deprecatedResult){
            result = result || deprecatedResult; // this is for possible old providers, which may had the error first handler

            // check for result.method, to prevent old providers errors to pass as result
            if(result.method && _this.subscriptions[result.params.subscription] && _this.subscriptions[result.params.subscription].callback) {
                _this.subscriptions[result.params.subscription].callback(null, result.params.result);
            }
        });
        // TODO add error, end, timeout, connect??
        // this.provider.on('error', function requestManagerNotification(result){
        //     Object.keys(_this.subscriptions).forEach(function(id){
        //         if(_this.subscriptions[id].callback)
        //             _this.subscriptions[id].callback(err);
        //     });
        // }
    }

that is called from packageInit:

pkg._requestManager.setProvider(args[0], args[1]);

that is called from Contract constructor:

 // sets _requestmanager
    core.packageInit(this, [this.constructor.currentProvider]);

This means setProvider is called for each contract constructed? If yes, then it means it's not possible to call new Contract on one page more than 101 times?

It is - it just warns you about a possible memory leak.

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

10 participants