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

Exit the process on successful download case #45

Closed
wants to merge 1 commit into from

Conversation

glemercier
Copy link

@glemercier glemercier commented Oct 23, 2017

When I use the following install command in package.json as recommended in the documentation:

...
  "scripts": {
    "install": "prebuild-install || node-gyp rebuild"
  }
...

The prebuilt binary is properly downloaded, installed, but after that the installation process never returns. Is that expected? If not, would this patch be an acceptable fix?

@vweevers
Copy link
Member

Sounds like there's something keeping the node process open. If so, doing process.exit(0) would hide a bug.

wtfnode can help to find open handles.

@glemercier
Copy link
Author

Here is what I get with wtfnode:

# wtfnode /usr/local/bin/npm install artik-sdk

> artik-sdk@1.6.1 install /root/node_modules/artik-sdk
> prebuild-install || node-gyp rebuild

prebuild-install info begin Prebuild-install version 2.3.0
prebuild-install info looking for local prebuild @ prebuilds/artik-sdk-v1.6.1-node-v48-linux-arm64.tar.gz
prebuild-install info looking for cached prebuild @ /root/.npm/_prebuilds/https-github.com-SamsungARTIK-artik-sdk-js-releases-download-v1.6.1-artik-sdk-v1.6.1-node-v48-linux-arm64.tar.gz
prebuild-install info found cached prebuild
prebuild-install info unpacking @ /root/.npm/_prebuilds/https-github.com-SamsungARTIK-artik-sdk-js-releases-download-v1.6.1-artik-sdk-v1.6.1-node-v48-linux-arm64.tar.gz
prebuild-install info unpack resolved to /root/node_modules/artik-sdk/build/Release/artik-sdk.node
prebuild-install info unpack required /root/node_modules/artik-sdk/build/Release/artik-sdk.node successfully
prebuild-install info install Successfully installed prebuilt binary!
^C[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Child processes
  - PID 366
    - Entry point: /usr/lib/node_modules/npm/lib/utils/spawn.js:21

Note sure what's wrong or if npm is to be blamed here. Note that I'm running on an ARM64 platform running Ubuntu 16.04 and here are is my setup in case it's relevant:

# node -v
v6.11.3
# npm -v
3.10.10

@glemercier
Copy link
Author

glemercier commented Oct 23, 2017

Same thing after upgrading to npm 5.5.1.

#npm -v
5.5.1
# wtfnode /usr/local/bin/npm install artik-sdk
...
prebuild-install info install Successfully installed prebuilt binary!
^C[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)
- Child processes
  - PID 442
    - Entry point: /usr/local/lib/node_modules/npm/node_modules/npm-lifecycle/lib/spawn.js:36

/usr/local/lib/node_modules/npm/node_modules/npm-lifecycle/lib/spawn.js

const _spawn = require('child_process').spawn
const EventEmitter = require('events').EventEmitter

let progressEnabled
let running = 0

function startRunning (log) {
  if (progressEnabled == null) progressEnabled = log.progressEnabled
  if (progressEnabled) log.disableProgress()
  ++running
}

function stopRunning (log) {
  --running
  if (progressEnabled && running === 0) log.enableProgress()
}

function willCmdOutput (stdio) {
  if (stdio === 'inherit') return true
  if (!Array.isArray(stdio)) return false
  for (let fh = 1; fh <= 2; ++fh) {
    if (stdio[fh] === 'inherit') return true
    if (stdio[fh] === 1 || stdio[fh] === 2) return true
  }
  return false
}

function spawn (cmd, args, options, log) {
  const cmdWillOutput = willCmdOutput(options && options.stdio)

  if (cmdWillOutput) startRunning(log)
  const raw = _spawn(cmd, args, options) =======================> line 36
  const cooked = new EventEmitter()

@vweevers
Copy link
Member

Might be more telling to use the programmatic API of wtfnode, so that it prints the handles of the process that's running prebuild-install.

Try adding it to prebuild-install/bin.js, with a require statement at the top and a dump() at the end:

#!/usr/bin/env node
var wtf = require('wtfnode')

// ..

log.info('install', 'Successfully installed prebuilt binary!')
wtf.dump()

@vweevers
Copy link
Member

Oh, could it be that your native module opens a handle / starts a timer / etc? Because prebuild-install does require('/path/to/artik-sdk.node') to test it, and this is the last step.

@glemercier
Copy link
Author

Did not get more info, actually less:

prebuild-install info install Successfully installed prebuilt binary!
[WTF Node?] open handles:
- File descriptors: (note: stdio always exists)
  - fd 1 (tty) (stdio)
  - fd 2 (tty) (stdio)

You're right about the test step, there are a lot of things going on when you require the "artik-sdk" module. Is there a way to bypass this verification step? Otherwise I'll look at ways to fix it in the addon.

@vweevers
Copy link
Member

Did not get more info, actually less.

That makes sense if the native module is the culprit.

Is there a way to bypass this verification step?

@ralphtheninja?

@ralphtheninja
Copy link
Member

Is there a way to bypass this verification step?

Nope, not at the moment. This sounds like a bug in the native node module, no module should lock if only required.

@glemercier
Copy link
Author

Makes sense, I'll go ahead and fix the native module. Thanks a lot guys for the quick support, closing the PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants