Merge #251

251: Fix hanging interface-ipfs-core tests r=ljedrz a=koivunej

This should add the diagnostic, but not hang the builds. Lets see how lucky we get. This is following the test win/mac failures on #249 and #250.

In this PR:
- why-is-node-running triggered 1min after interface-ipfs-core tests complete (so visible only when stuck)
- ipfsd-ctl patched to always await for subprocesses (this seems healthy either way)
- tests ignored:
    - miscellanious.stop for surely causing the hang
    - add from invalid url for likely causing a hang (DNSWRAP)

Need to investigate further what is causing the hangs with the stop -- it seemed like a deadlock. Not sure what happens with the DNSWRAP; perhaps the invalid url is good enough, but our rejection causes it to be "forgotten" and as such it seems to linger as it's completion is never checked once we fail the url add.

Co-authored-by: Joonas Koivunen <joonas@equilibrium.co>

.github/workflows/ci.yml

@@ -56,6 +56,7 @@ jobs:
       LLVM_CONFIG_PATH: /usr/local/opt/llvm/bin/llvm-config
       NDK_HOME: /usr/local/lib/android/sdk/ndk-bundle
       VCPKGRS_DYNAMIC: 1
+      DEBUG: ipfsd-ctl:* # enables all debug output from javascript 'debug' lib used by js-ipfsd-ctl
 
     runs-on: ${{ matrix.platform.host }}
     steps:

conformance/package-lock.json

@@ -19418,6 +19418,11 @@
         }
       }
     },
+    "siginfo": {
+      "version": "2.0.0",
+      "resolved": "https://registry.npmjs.org/siginfo/-/siginfo-2.0.0.tgz",
+      "integrity": "sha512-ybx0WO1/8bSBLEWXZvEd7gMW3Sn3JFlW3TvX1nREbDLRNQNaeNN8WK0meBwPdAaOI7TtRRRJn/Es1zhrrCHu7g=="
+    },
     "signal-exit": {
       "version": "3.0.3",
       "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.3.tgz",
@@ -19970,6 +19975,11 @@
       "resolved": "https://registry.npmjs.org/stable/-/stable-0.1.8.tgz",
       "integrity": "sha512-ji9qxRnOVfcuLDySj9qzhGSEFVobyt1kIOSkj1qZzYLzq7Tos/oUUWvotUPQLlrsidqsK6tBH89Bc9kL5zHA6w=="
     },
+    "stackback": {
+      "version": "0.0.2",
+      "resolved": "https://registry.npmjs.org/stackback/-/stackback-0.0.2.tgz",
+      "integrity": "sha1-Gsig2Ug4SNFpXkGLbQMaPDzmjjs="
+    },
     "state-toggle": {
       "version": "1.0.3",
       "resolved": "https://registry.npmjs.org/state-toggle/-/state-toggle-1.0.3.tgz",
@@ -22306,6 +22316,15 @@
       "resolved": "https://registry.npmjs.org/which-pm-runs/-/which-pm-runs-1.0.0.tgz",
       "integrity": "sha1-Zws6+8VS4LVd9rd4DKdGFfI60cs="
     },
+    "why-is-node-running": {
+      "version": "2.2.0",
+      "resolved": "https://registry.npmjs.org/why-is-node-running/-/why-is-node-running-2.2.0.tgz",
+      "integrity": "sha512-rxtN9D0lJaYyP92BR5yoyWecK2txBKmBIuS7GRbOPP5bXsT37/hBqcmTrlrt25DBr9p4WJb6c9LuYSJd89vHRQ==",
+      "requires": {
+        "siginfo": "^2.0.0",
+        "stackback": "0.0.2"
+      }
+    },
     "wide-align": {
       "version": "1.1.3",
       "resolved": "https://registry.npmjs.org/wide-align/-/wide-align-1.1.3.tgz",

conformance/package.json

@@ -13,7 +13,8 @@
     "interface-ipfs-core": "0.137.0",
     "ipfsd-ctl": "github:ipfs-rust/js-ipfsd-ctl#add_rust_ipfs_http",
     "mocha": "^7.1.0",
-    "ipfs-http-client": "^44.3.0"
+    "ipfs-http-client": "^44.3.0",
+    "why-is-node-running": "2.2.0"
   },
   "devDependencies": {
     "rust-ipfs-dep": "github:ipfs-rust/npm-rust-ipfs-dep#master",

conformance/patches/0001-fix-allow-either-cidv-0-1-in-refs-local-test.patch

@@ -14,8 +14,8 @@ implementation to use either version internally.
 
 diff --git a/packages/interface-ipfs-core/src/refs-local.js b/packages/interface-ipfs-core/src/refs-local.js
 index d20851de..33a94c74 100644
---- a/packages/interface-ipfs-core/src/refs-local.js
-+++ b/packages/interface-ipfs-core/src/refs-local.js
+--- a/interface-ipfs-core/src/refs-local.js
++++ b/interface-ipfs-core/src/refs-local.js
 @@ -7,6 +7,7 @@ const all = require('it-all')
  const importer = require('ipfs-unixfs-importer')
  const drain = require('it-drain')

conformance/patches/await_subprocess.patch

@@ -0,0 +1,10 @@
+--- node_modules/ipfsd-ctl/src/ipfsd-daemon.js	2020-03-06 15:26:44.365192780 +0200
++++ node_modules/ipfsd-ctl/src/ipfsd-daemon.js	2020-07-20 14:54:43.353680062 +0300
+@@ -260,6 +250,7 @@
+
+     try {
+       await this.api.stop()
++      await this.subprocess;
+     } catch (err) {
+       if (!killed) {
+         throw err // if was killed then ignore error

conformance/setup.sh

@@ -21,6 +21,6 @@ if [ -d "patches" ]; then
     # and node_modules are ignored.
     for p in patches/*; do
         echo "Applying $(basename "$p")..." >&2
-        patch -d node_modules/interface-ipfs-core/ -p3 < "$p"
+        patch -d node_modules/ -p1 < "$p"
     done
 fi

conformance/test/index.js

@@ -1,3 +1,4 @@
+const log = require('why-is-node-running')
 const { createFactory } = require('ipfsd-ctl')
 const tests = require('interface-ipfs-core')
 const isDev = process.env.IPFS_RUST_EXEC
@@ -8,6 +9,12 @@ const ipfsBin = isNode ?
   process.env.IPFS_RUST_EXEC ? process.env.IPFS_RUST_EXEC : require('rust-ipfs-dep').path()
     : undefined
 
+after(() => {
+  const whyIsNodeRunning = setTimeout(() => log(), 1000 * 60)
+  // this should not block shutting down
+  whyIsNodeRunning.unref()
+})
+
 const options = {
   type: 'rust',
   ipfsBin,
@@ -30,7 +37,9 @@ tests.miscellaneous(factory, { skip: [
   'resolve',
   // these cause a hang 20% of time:
   'should respect timeout option when getting the node id',
-  'should respect timeout option when getting the node version'
+  'should respect timeout option when getting the node version',
+  // this hangs on windows, see #251
+  'stop'
 ] })
 
 // Phase 1.1
@@ -99,7 +108,10 @@ tests.root.add(factory, {
     "should add from a HTTP URL with redirection",
     "should add from a URL with only-hash=true",
     "should add from a URL with wrap-with-directory=true",
-    "should add from a URL with wrap-with-directory=true and URL-escaped file name"
+    "should add from a URL with wrap-with-directory=true and URL-escaped file name",
+    // this might hang on windows at least; seems that there is a DNSCHANNEL open
+    // see https://github.com/rs-ipfs/rust-ipfs/pull/251/checks?check_run_id=889139927#step:17:934
+    "should not add from an invalid url"
   ]
 });
 // tests.repo(factory)

http/src/main.rs

@@ -172,6 +172,8 @@ fn main() {
                 .map_err(|e| eprintln!("Failed to truncate {:?}: {}", api_link_file, e));
         }
     });
+
+    println!("Shutdown complete");
 }
 
 fn serve<Types: IpfsTypes>(
@@ -188,6 +190,7 @@ fn serve<Types: IpfsTypes>(
 
     warp::serve(routes).bind_with_graceful_shutdown(([127, 0, 0, 1], 0), async move {
         shutdown_rx.next().await;
+        println!("Shutdown trigger received; starting shutdown");
         ipfs.exit_daemon().await;
     })
 }