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 closure leak in web3.eth.Contract 1.x ? #3042

Closed
ngutman opened this issue Aug 20, 2019 · 24 comments
Closed

Possible closure leak in web3.eth.Contract 1.x ? #3042

ngutman opened this issue Aug 20, 2019 · 24 comments
Labels
1.x 1.0 related issues 2.x 2.0 related issues Bug Addressing a bug Stale Has not received enough activity

Comments

@ngutman
Copy link

ngutman commented Aug 20, 2019

Description

Expected behavior

Memory usage should stay low over time when instantiating Contract objects.

Actual behavior

After investigating the heap there seems to be a closure leak which increases memory use over time until the process stops.

Steps to reproduce the behavior

You can run the following sample code with the gc exposed

const Web3 = require('web3')

async function main() {
  const web3 = new Web3(new Web3.providers.HttpProvider('https://mainnet.infura.io'))

  for (let i = 0; ;i++) {
    new web3.eth.Contract([], '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2')
    if ((i % 1000) === 0) {
      // Use node --expose-gc ./testLeak.js
      if (global.gc) { global.gc() }
      const used = process.memoryUsage().heapUsed / 1024 / 1024
      console.log(`Heap used ${used}`)
    }
  }
}

main()

You'll see the memory rapidly increasing:

node --expose-gc ./testLeak.js
.....
Heap used 120.24988555908203
Heap used 124.3317642211914
Heap used 128.3788604736328
Heap used 132.4272689819336
Heap used 136.45621490478516
Heap used 140.50664520263672
Heap used 144.50611877441406
Heap used 148.57512664794922
Heap used 152.59766387939453
Heap used 156.6605682373047
Heap used 160.72467803955078
Heap used 164.76478576660156
Heap used 168.77734375
Heap used 172.78372192382812
Heap used 176.8772201538086
Heap used 180.86630249023438
....

This is the commit that introduced the issue 26a8775 (apologies for the direct "blame" it's just easier to direct to the actual code with the issue)

Removing the setProvider chaining solves the leak.

Error Logs

Gists

Versions

  • web3.js: v1.2.1
  • nodejs: v10.16.0
  • ethereum node: infura mainnet
@ngutman ngutman changed the title Possible closure leak in 1.x ? Possible closure leak in web3.eth.Contract 1.x ? Aug 20, 2019
@mgsgde
Copy link

mgsgde commented Aug 29, 2019

Same problem with "web3.js: v2.0.0-alpha". In this version it seems to be not due to a closure leak, but probably due to the following line: https://github.com/ethereum/web3.js/blob/2.x/packages/web3-eth/src/Eth.js#L107

Version

  • web3.js: v2.0.0-alpha
  • nodejs: v10.16.3
  • ethereum node: truffle dev network
  • truffle: v5.0.16

@nivida
Copy link
Contributor

nivida commented Sep 5, 2019

@mgsgde Yes, it does store these contract for being able later to update the properties and providers from the contracts as well if you update them on the Eth module.

@ngutman The creation of new contract objects in the for loop does increase the memory usage.

@nivida nivida closed this as completed Sep 5, 2019
@mgsgde
Copy link

mgsgde commented Sep 7, 2019

Hey Samuel, thx for the quick answer. web3.eth.net.isListening() does also seem to increase the memory.

Version

  • web3.js: 1.2.1
  • nodejs: v10.16.3
  • ethereum node: truffle dev network
  • truffle: v5.0.16

@ngutman
Copy link
Author

ngutman commented Sep 15, 2019

Thanks for the response @nivida.
Thought not sure I fully understand why the issue was closed - It's not that trivial that memory footprint increases when creating new contract objects (after the gc cleans them up when there are no references to them).

I'm certain that numerous production systems that use web3.js are affected by it, like we were, it's fully understandable there are issues since the project development is very active and production systems shouldn't expect web3.js to be fully stable. I just wonder if there's anything else we can do to mitigate the implications - if this is the expected behaviour (memory footprint increase) I think it should be documented somewhere, although IMO this behaviour is not very trivial.

@sirasistant
Copy link

This is creating an issue for us, since we instantiate contract objects on demand and get rid of them in long running operations. Why not using WeakMaps to make sure that you only update contracts that are still held by the user? or maybe providing some destroy functionality in web3.eth.Contract in order to at least be able to clean the instances once we don't care about them any more?

@cgewecke cgewecke reopened this Oct 10, 2019
@nivida nivida added 1.x 1.0 related issues Bug Addressing a bug 2.x 2.0 related issues labels Oct 13, 2019
@nivida nivida mentioned this issue Nov 21, 2019
7 tasks
@github-actions
Copy link

github-actions bot commented Jul 3, 2020

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment, otherwise this issue will be closed in 7 days

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 3, 2020
@cgewecke
Copy link
Collaborator

cgewecke commented Jul 3, 2020

Not stale

@cgewecke cgewecke removed the Stale Has not received enough activity label Jul 3, 2020
@balthazar
Copy link

Watching this too

@dylanseago
Copy link

I've discovered a workaround:

const Web3 = require('web3')
const Contract = require('web3-eth-contract')

async function main() {
  const web3 = new Web3(new Web3.providers.HttpProvider('https://mainnet.infura.io'))

  for (let i = 0; ;i++) {
    const contract = new Contract([], '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2')
    contract.setProvider(web3.currentProvider)
    if ((i % 1000) === 0) {
      // Use node --expose-gc ./testLeak.js
      if (global.gc) { global.gc() }
      const used = process.memoryUsage().heapUsed / 1024 / 1024
      console.log(`Heap used ${used}`)
    }
  }
}

main()

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Sep 23, 2020
@dylanseago
Copy link

Not stale, still an issue

@balthazar
Copy link

Not stale

@github-actions github-actions bot removed the Stale Has not received enough activity label Sep 24, 2020
@kigorw
Copy link

kigorw commented Nov 23, 2020

not stale, please fix

@immasandwich
Copy link

Not stale!

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Feb 10, 2021
@balthazar
Copy link

reopen

@chancehudson
Copy link

This issue needs to be reopened. There is an open PR with a fix in #3866.

@pmprete
Copy link

pmprete commented Aug 3, 2021

Still an issue

marcciosilva pushed a commit to rsksmart/tokenbridge that referenced this issue Aug 4, 2021
…ap class variable) to avoid web3.eth.Contract memory leak issue (web3/web3.js#3042), replace var with let/const where appropriate
@0xCheeseKnight
Copy link

This issue was crippling our servers due to needing to programatically interact with a lot of contracts. We have switched over to the ethers library for instantiating contracts and the leak is now gone. Hopefully this gets fixed soon.

@ko0f
Copy link

ko0f commented Jun 3, 2022

This memory leak still going on, had to switch our project to ethers.

@Adeel-trimulabs
Copy link

Any update or workaround this?

@OsoianMarcel
Copy link

Any update or workaround this?

Did you read all the messages?
#3042 (comment)

@bvanderdrift
Copy link

The workaround seems to not be possible anymore now that setProvider has become a static function on the Contract class instead of the instance.

I've also migrated to ethers and can recommend it. Did a little writeup here for whoever is interested.

Effort & Results

It took about 12 hours to migrate a medium-complexity codebase from web3 to ethers, including unit tests. Things have calmed down since:

web3 charts:
Screenshot 2023-02-02 at 20 43 01

ethers has more stable CPU usage & no more memory leak. I did no other refactors than just move over to ethers:
Screenshot 2023-02-02 at 21 52 40

@adrian-gierakowski
Copy link

Wow, this makes me really confident about the Ethereum ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues 2.x 2.0 related issues Bug Addressing a bug Stale Has not received enough activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.