-
Notifications
You must be signed in to change notification settings - Fork 445
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 EventTarget Memory Leak Detected #900
Comments
Thanks for reporting this @TJKoury Can you share a code snippet and dependencies for this or a replication test directly with libp2p? From https://github.com/libp2p/js-libp2p/blob/master/src/dialer/dial-request.js#L70 + https://github.com/jacobheun/any-signal/blob/master/index.js#L27 I cannot see at this point how 11 listeners would be added. |
See snippet below. Can you recommend a test harness setup for libp2p? I was trying to follow the dialog in issue 886. Tried using libp2p-daemon but could not get it to run after global install.
|
There are some inconsistencies in the snippet,
Perhaps it is easier to just import libp2p directly and use its API? |
Just fixed the snippet, it was part of a larger test with multiple keys, I
just tried it by itself.
Will try your suggestion for libp2p-daemon.
…On Mon, Mar 15, 2021 at 5:16 AM Vasco Santos ***@***.***> wrote:
There are some inconsistencies in the snippet, peerId1 is not set and the
crypto lib not used. For now, I just removed the privatekey set.
Do you get the above warning by running this?
Tried using libp2p-daemon but could not get it to run after global install.
Perhaps it is easier to just import libp2p directly and use its API?
libp2p-daemon should just need to be imported, a daemon created and
started, but you also need libp2p-daemon-client to interact with it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#900 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFW4KFUM7UD6CW7UTB5LLLTDXF5VANCNFSM4Y7H2G5Q>
.
|
Thanks @TJKoury |
I have been running it for 2 hours without any issue. Perhaps linux related as I am running on Mac OS. |
I will boot up my Mac (2014 MBP) and see if I can replicate.
…On Mon, Mar 15, 2021 at 7:34 AM Vasco Santos ***@***.***> wrote:
I have been running it for 2 hours without any issue. Perhaps linux
related as I am running on Mac OS.
Probably also related to libp2p/js-libp2p-tcp#141
<libp2p/js-libp2p-tcp#141>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#900 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFW4KE7ATZ6QQXHD73QKTTTDXWEFANCNFSM4Y7H2G5Q>
.
|
OSX Mojave 10.14.6, Node v15.11.0, no issues. Will try on Windows next. Test repo. |
Interesting, can you also try your linux machine using node14? |
Both are using Node 15, so it seems it might be platform related. If there is another reason you think I should try let me know. |
I wanted to understand if this might be related to an update on Node15 in the platform you are having issues, as this problem should have appeared early on if it is a problem in other Node versions. If you can, it would be great to test your machine using Node14. Meanwhile, I will try to book some time to try to replicate this on a VM |
I'm seeing this in js-IPFS master too. To repro just start the daemon and wait a few seconds. |
@achingbrain are you using Node15 + MacOS? I have been running it for over two hours and I only got:
|
Yes, node 15 on a Mac:
|
It's trying to dial more than 10 addresses at once and an // added to src/dialer/dial-request.js
const dialAbortControllers = this.addrs.map(() => new AbortController())
let completedDials = 0
console.info(dialAbortControllers.length, 'controllers', this.addrs)
|
We allow by default doing 4 parallel dials per dialRequest. So after 4 being in progress, the next ones will wait for a token to start dialling. Anyway, I already understood what is happening. In this case, it seems that before an actual dial is successful a bunch are tested. As we we do not support quic we will add abort controllers to it, even though we will not be able to dial it. So, we will rapidly go over all the quic addresses, while the TCP will actually do the dials. This means the There is definitely room for improvements here! Probably the cleanest solution will be to call transportManager. transportForMultiaddr() while creating a DialTarget. This way we can immediately filter out the multiaddrs that will be useless before we use dial tokens with them and create AbortControllers. The downside of this approach is that we already call I am going to add this issue to the maintenance board to get it fixed faster. Thanks for your inputs |
I'm still seeing this happen with
It's trying to dial a peer with more than 10x addresses:
We just need to handle peers like this. Put the dials in a queue or increase the number of listeners or something. The warning is harmless, but it doesn't look harmless. |
Sometimes you encounter peers with lots of addresses. When this happens you can attach more than 10x event listeners to the abort signal we use to abort all the dials - this causes node to print a warning which is misleading. This PR increases the default number of listeners on the signal. Fixes #900
Sometimes you encounter peers with lots of addresses. When this happens you can attach more than 10x event listeners to the abort signal we use to abort all the dials - this causes node to print a warning which is misleading. This PR increases the default number of listeners on the signal. Fixes #900
Version: 0.30.10
Platform: Linux pop-os 5.8.0-7642-generic # 47~ 1612288990 ~20.10 ~b8113e7-Ubuntu x86_64 GNU/Linux, Node v15.8.0.
Subsystem: Dialer (src/dialer/dial-request.js)
Type: Possible EventTarget memory leak detected.
Severity: Medium
Description:
(node:52434) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal]. Use events.setMaxListeners() to increase limit
at EventTarget.[kNewListener] (node:internal/event_target:256:17)
at EventTarget.addEventListener (node:internal/event_target:335:23)
at anySignal (node_modules/any-signal/index.js:27:12)
at node_modules/libp2p/src/dialer/dial-request.js:70:85
at runMicrotasks ()
at processTicksAndRejections (node:internal/process/task_queues:94:5)
Steps to reproduce the error:
Import
ipfs-core
into a Node project, initialize, let it run for ~30 minutes.The text was updated successfully, but these errors were encountered: