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

5704 memory leak in web3.eth.contract #5792

Merged
merged 12 commits into from
Jan 31, 2023

Conversation

avkos
Copy link
Contributor

@avkos avkos commented Jan 30, 2023

Description

Fixes #5704

So the problem was a new SubscriptionManager which was created every time when making a new instance of the Contract class because, by default, web3 didn't create the registeredSubscriptions object. So I've added the object by default. And now it always reuses the same SubscriptionManager in the contract class.

Also, I've removed the array of contracts in the web3 package. Because we don't need it. When we set the provider in one of the contracts or in web3, it affects all instances.

After these changes memory leak disappears.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist for 4.x:

  • I have selected the correct 4.x base branch.
  • Within the description, the feature or issue is discussed in detail or an issue is linked.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added any required tests for the changes I made
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged and published in downstream modules.
  • I ran yarn successfully
  • I ran yarn lint successfully
  • I ran yarn build:web successfully
  • I ran yarn test:unit successfully
  • I ran yarn test:integration successfully
  • I ran compile:contracts successfully
  • I have tested my code.
  • I have updated the corresponding CHANGELOG.md file in the packages I have edited.

@avkos avkos changed the title Ok/5704 memory leak in web3.eth.contract 5704 memory leak in web3.eth.contract Jan 30, 2023
@github-actions github-actions bot temporarily deployed to Preview: (ok/5704-Memory-leak-in-web3.eth.Contract) January 30, 2023 18:36 Inactive
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 30, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: c6026b8
Status: ✅  Deploy successful!
Preview URL: https://649e9e5c.web3-js-docs.pages.dev
Branch Preview URL: https://ok-5704-memory-leak-in-web3-.web3-js-docs.pages.dev

View logs

@github-actions github-actions bot temporarily deployed to Preview: (ok/5704-Memory-leak-in-web3.eth.Contract) January 30, 2023 20:28 Inactive
@github-actions github-actions bot temporarily deployed to Preview: (ok/5704-Memory-leak-in-web3.eth.Contract) January 30, 2023 21:30 Inactive
@@ -91,16 +88,6 @@ export class Web3 extends Web3Context<EthExecutionAPI> {
} else {
super(jsonInterface, self.getContextObject());
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also update change log for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@jdevcs jdevcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, except a minor change log work.

@jdevcs jdevcs added the 4.x 4.0 related label Jan 31, 2023
@github-actions github-actions bot temporarily deployed to Preview: (ok/5704-Memory-leak-in-web3.eth.Contract) January 31, 2023 15:00 Inactive
@github-actions github-actions bot temporarily deployed to Preview: (ok/5704-Memory-leak-in-web3.eth.Contract) January 31, 2023 15:07 Inactive
CHANGELOG.md Outdated

##### Removed

- Private static `_contracts:Contract[]` and static `setProvider` function was removed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add PR/Issue number

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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

Successfully merging this pull request may close these issues.

4 participants