-
Notifications
You must be signed in to change notification settings - Fork 447
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
[switch] ensure all connections are closed on switch stop #436
Comments
Example script: const IPFS = require('ipfs')
const Path = require('path')
const Os = require('os')
const log = require('why-is-node-running')
const ipfs = new IPFS({ repo: Path.join(Os.tmpdir(), `${Date.now()}`) })
ipfs.on('ready', async () => {
const files = Array.from([1, 2, 3], i => ({
path: `test-file-${Date.now()}-${i}`,
content: Buffer.from(`data ${Date.now()} ${i}`)
}))
const res = await ipfs.files.add(files, { wrapWithDirectory: true })
console.log('Added files:')
console.log(res)
const { hash } = res[res.length - 1]
const ipfsPath = `/ipfs/${hash}`
console.log(`Copying from ${ipfsPath}`)
const mfsPath = `/test-dir-${Date.now()}`
console.log(`Copying to ${mfsPath}`)
await ipfs.files.cp(ipfsPath, mfsPath)
console.log(`Directory listing of ${mfsPath}:`)
console.log(await ipfs.files.ls(mfsPath))
await ipfs.stop()
setTimeout(() => log(), 10000)
}) |
There are timeouts in latency-monitor, but all refs are removed on shutdown. There are private methods on the latency-monitor for turning off the timers, ideally we'd be able to shut those down, but it shouldn't be an issue for this. The
|
@alanshaw it looks like the |
Preloading requests are tracked and canceled when the node is stopped https://github.com/ipfs/js-ipfs/blob/master/src/core/preload.js#L76-L81. Unless something has changed in a new Node.js version I don't think these are problematic as that code hasn't changed now for 3 months. This is a very recent change. I noticed because all the examples here used to exit when done but now don't :( |
Okay, I can replicate that now with the |
After look at this more there are a few things we're going to need to make sure are happening for graceful shutdowns, several of which aren't currently happening. TODO List
|
This has marginally improved with 0.41.3. Closing out in progress dials is the major issue left to resolve. |
@jacobheun can you post the links to the PRs that are solving the issues you listed in https://github.com/libp2p/js-libp2p-switch/issues/281#issuecomment-432624680? |
I added issues that were missing to complete this work and updated the todo list in the comment, https://github.com/libp2p/js-libp2p-switch/issues/281#issuecomment-447384312. There are several issues that need to be completed for in progress/queued dials, so it's still a significant amount of work left. The marginal improvement was the result of a fixed bug where not all connections were being properly closed, libp2p/js-libp2p-switch#291. They are now being tracked better, and are also closing the underlying duplex stream, which they weren't previously doing. |
## [8.0.4](libp2p/js-libp2p-kad-dht@v8.0.3...v8.0.4) (2023-03-30) ### Bug Fixes * correction package.json exports types path ([libp2p#436](libp2p/js-libp2p-kad-dht#436)) ([5024646](libp2p/js-libp2p-kad-dht@5024646))
Closing due to staleness |
I'm using
why-is-node-running
to inspect what's causing node to keep running afteripfs.stop()
is called. I think the "latency-monitor" one is a red herring as these timeouts are unref'd:The text was updated successfully, but these errors were encountered: