Skip to content

interface-ipfs-core with patched ipfsd-ctl 3.0.0 leaves a lot of PIPEWRAP behind #525

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

Closed
koivunej opened this issue Jul 20, 2020 · 6 comments
Labels
need/triage Needs initial labeling and prioritization

Comments

@koivunej
Copy link

We recently had mac and windows builds start hanging on the "conformance" tests (just interface-ipfs-core for now). I've added why-is-node-running in rs-ipfs/rust-ipfs#251 (on 60s unref'd timer once mocha finishes) and it seems that for 155 passing tests there are 90 PIPEWRAP instances (possibly) blocking node from exiting.

This is the log of the first run hanging run: https://github.com./rs-ipfs/rust-ipfs/pull/251/checks?check_run_id=889139927#step:17:288

  • there are 90 PIPEWRAP which I think are for improperly shutdown nodes
  • 1 DNSCHANNEL which might be the real reason for blocking

A PIPEWRAP looks like:

There are 91 handle(s) keeping the process running

# PIPEWRAP
D:\a\rust-ipfs\rust-ipfs\conformance\node_modules\execa\index.js:80                              - spawned = childProcess.spawn(parsed.file, parsed.args, parsed.options);
D:\a\rust-ipfs\rust-ipfs\conformance\node_modules\ipfsd-ctl\src\ipfsd-daemon.js:188              - this.subprocess = execa(this.exec, args, {
D:\a\rust-ipfs\rust-ipfs\conformance\node_modules\ipfsd-ctl\src\ipfsd-daemon.js:187              - const ready = new Promise((resolve, reject) => {
D:\a\rust-ipfs\rust-ipfs\conformance\node_modules\ipfsd-ctl\src\factory.js:142                   - await ctl.start()
D:\a\rust-ipfs\rust-ipfs\conformance\node_modules\interface-ipfs-core\src\miscellaneous\id.js:23 - ipfs = (await common.spawn()).api

The 2nd commit in the rs-ipfs/rust-ipfs#251 ignores the test which creates the dns query which I guess is never reaped as of now.

Also I now notice that you've made a lot progress with this package and we are stuck with patched 3.0.0. Apologies up front if I cannot see the above being fixed in any of the changelog entries. We can try with a patched 5.0 to see if they get rid of those PIPEWRAPs.

I wonder if #509 is related?

@koivunej koivunej added the need/triage Needs initial labeling and prioritization label Jul 20, 2020
@koivunej
Copy link
Author

Still hanging without the DNSCHANNEL, so I am guessing that the cleanup fails. However it is strange that there are no test failures because of it nor and no how to isolate these hangs.

@koivunej
Copy link
Author

Tried to add a await this.subprocess after https://github.com./ipfs/js-ipfsd-ctl/blob/master/src/ipfsd-daemon.js#L250 (the api.stop()) call and it seems to cause the PIPEWRAPs to go away, which sounds quite natural. Trying to see if this helps to identity the hanging test case in our builds.

@koivunej
Copy link
Author

Awaiting for the subprocesses definitely helps. Now there's only 8 left on our latest windows build with a hack patch to just await this.subprocess on src/ipfsd-daemon.js after this.api.stop() call. Latest log: https://github.com./rs-ipfs/rust-ipfs/pull/251/checks?check_run_id=889710656#step:17:4268

@jacobheun
Copy link
Contributor

@hugomrdias can you take a look at this?

@koivunej
Copy link
Author

With #526 now closed after a long while, I think this could be closed as a sort of invalid report. While it's undeniable that these used to happen on github ci builds, I haven't tried out if they happen for example after the ubuntu 20.04 upgrade.

@lidel
Copy link
Member

lidel commented Jun 7, 2021

Assuming #526 closed the problem, please fill a new one with more actionable details if you feel the variant of this problem is still present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants