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

bug: Memory leak using watchEvent #583

Closed
1 task done
adriencohen opened this issue May 25, 2023 · 11 comments
Closed
1 task done

bug: Memory leak using watchEvent #583

adriencohen opened this issue May 25, 2023 · 11 comments
Labels
A: Actions Area: Actions D: Medium Difficulty: Medium P: Medium Priority: Medium T: Bug Type: Bug

Comments

@adriencohen
Copy link

adriencohen commented May 25, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Package Version

0.3.36

Current Behavior

Using the watchEvent function is leading to a growing memory consumption which can only be stopped using the unwatch function returned by watchEvent. Memory consumption will then stagnate.

Expected Behavior

Memory consumption shouln't be evergrowing

Steps To Reproduce

A quick example running the following code will produce the graph attached after:

import { createPublicClient } from 'viem';
import { mainnet } from 'viem/chains';

const client = createPublicClient({
  chain: mainnet,
  transport: http(),
});

setTimeout(() => {
  const unwatch = client.watchEvent(
    { 
      onLogs: () => {},
    }
  ) 
  console.log('watch')


  setTimeout(() => {
    unwatch()
    console.log('Unwatched')
  }, 300000)

}, 300000)


setInterval(() => {
    console.log('Memory used:', process.memoryUsage().heapUsed / 1024 / 1024);
}, 10000)

Using http transport (node v18.12.1):
Capture d’écran 2023-05-25 à 15 22 00

Using ws transport (node v18.12.1):
Capture d’écran 2023-05-25 à 16 00 47

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

No response

Anything else?

No response

@jxom jxom added A: Actions Area: Actions D: Medium Difficulty: Medium P: Medium Priority: Medium T: Bug Type: Bug labels May 26, 2023
@fubhy
Copy link
Collaborator

fubhy commented May 26, 2023

I am seeing similar results in other observers too. So likely not related to watchEvent itself but underlying utils.

@jxom
Copy link
Member

jxom commented May 26, 2023

Think I found the culprit. It's in our withTimeout util. Need to find out what's causing it though.

@jxom
Copy link
Member

jxom commented May 26, 2023

can you try see if this still happens on npm i viem@0.0.0-jxom-fix-memory-leak.20230526T074413?

the memory leak seems to be gone completely when i remove abort controllers, seems like we need to clean them up: 6be7922#diff-8b7a876b21ac77521fcb4a60160e0baceb18c2415682748d306b0d2fb445c480R35

@fubhy
Copy link
Collaborator

fubhy commented May 26, 2023

I don't think it's about cleaning up the AbortController and instead more about cleaning up the lingering fetch?

@jxom
Copy link
Member

jxom commented May 26, 2023

What do you mean? When I remove the AbortController completely, I no longer see a memory leak.

@jxom
Copy link
Member

jxom commented May 26, 2023

Might be an issue with how built-in fetch handles signals: nodejs/node#46525 (comment)

@fubhy
Copy link
Collaborator

fubhy commented May 26, 2023

True. But when I run a script with short timeouts the memory still goes up so I suspect there's sth. with fetch?

@fubhy
Copy link
Collaborator

fubhy commented May 26, 2023

Might be an issue with how built-in fetch handles signals: nodejs/node#46525 (comment)

Good find. Maybe that's what I'm seeing?

@fubhy
Copy link
Collaborator

fubhy commented May 26, 2023

can you try see if this still happens on npm i viem@0.0.0-jxom-fix-memory-leak.20230526T074413?

the memory leak seems to be gone completely when i remove abort controllers, seems like we need to clean them up: 6be7922#diff-8b7a876b21ac77521fcb4a60160e0baceb18c2415682748d306b0d2fb445c480R35

Can confirm that works to also clean up the lingering fetchs.

@jxom
Copy link
Member

jxom commented May 26, 2023

Looks like this was an issue with previous versions of Node, and was fixed downstream here: nodejs/undici#2049. Also related: #385 (reply in thread)

I tried again on Node v20, and it seems to work fine.

Solution: Update Node to >=20.1.0. No change needed on viem side.

Copy link
Contributor

github-actions bot commented Jun 2, 2024

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest Viem version. If you have any questions or comments you can create a new discussion thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: Actions Area: Actions D: Medium Difficulty: Medium P: Medium Priority: Medium T: Bug Type: Bug
Projects
None yet
Development

No branches or pull requests

3 participants