fix(server): terminate the store server if graceful stop fails (#1003)

* refactor: use process-exists instead of is-running

* fix(server): terminate the store server if graceful stop fails

package.json

@@ -37,6 +37,7 @@
     "chalk": "^2.2.0",
     "common-tags": "^1.4.0",
     "cross-spawn": "^5.0.0",
+    "delay": "^2.0.0",
     "diable": "^4.0.1",
     "find-packages": "^2.1.2",
     "get-port": "^3.2.0",
@@ -56,13 +57,16 @@
     "pnpm-default-reporter": "^0.11.8",
     "pnpm-file-reporter": "^0.0.1",
     "pnpm-list": "^2.0.0",
+    "process-exists": "^3.0.0",
     "ramda": "^0.25.0",
     "retry": "^0.10.1",
     "signal-exit": "^3.0.2",
     "strip-color": "^0.1.0",
     "supi": "^0.12.0",
     "text-table": "^0.2.0",
+    "tree-kill": "^1.2.0",
     "update-notifier": "^2.1.0",
+    "util.promisify": "^1.0.0",
     "write-json-file": "^2.3.0"
   },
   "devDependencies": {
@@ -84,7 +88,6 @@
     "@zkochan/husky": "^0.0.0",
     "anonymous-npm-registry-client": "^0.1.2",
     "caw": "^2.0.0",
-    "delay": "^2.0.0",
     "execa": "^0.9.0",
     "exists-link": "^2.0.0",
     "isexe": "^2.0.0",
@@ -105,8 +108,6 @@
     "sepia": "^2.0.2",
     "tape": "^4.6.3",
     "tape-promise": "^2.0.1",
-    "thenify": "^3.3.0",
-    "tree-kill": "^1.2.0",
     "ts-node": "^4.0.0",
     "tslint": "^5.4.2",
     "typescript": "^2.4.1",

shrinkwrap.yaml

@@ -14,6 +14,7 @@ dependencies:
   chalk: 2.3.0
   common-tags: 1.7.2
   cross-spawn: 5.1.0
+  delay: 2.0.0
   diable: 4.0.1
   find-packages: 2.1.2
   get-port: 3.2.0
@@ -33,13 +34,16 @@ dependencies:
   pnpm-default-reporter: 0.11.8
   pnpm-file-reporter: 0.0.1
   pnpm-list: 2.0.1
+  process-exists: 3.0.0
   ramda: 0.25.0
   retry: 0.10.1
   signal-exit: 3.0.2
   strip-color: 0.1.0
   supi: 0.12.0
   text-table: 0.2.0
+  tree-kill: 1.2.0
   update-notifier: 2.3.0
+  util.promisify: 1.0.0
   write-json-file: 2.3.0
 devDependencies:
   '@commitlint/cli': 4.3.0
@@ -60,7 +64,6 @@ devDependencies:
   '@zkochan/husky': 0.0.0
   anonymous-npm-registry-client: 0.1.2
   caw: 2.0.1
-  delay: 2.0.0
   execa: 0.9.0
   exists-link: 2.0.0
   isexe: 2.0.0
@@ -81,8 +84,6 @@ devDependencies:
   sepia: 2.0.2
   tape: 4.8.0
   tape-promise: 2.0.1
-  thenify: 3.3.0
-  tree-kill: 1.2.0
   ts-node: 4.1.0
   tslint: 5.9.1
   typescript: 2.6.2
@@ -1781,6 +1782,16 @@ packages:
       node: '>=4'
     resolution:
       integrity: sha1-ojD2T1aDEOFJgAmUB5DsmVRbyn4=
+  /csv-parser/1.12.0:
+    dependencies:
+      generate-function: 1.1.0
+      generate-object-property: 1.2.0
+      inherits: 2.0.3
+      minimist: 1.2.0
+      ndjson: 1.5.0
+    dev: false
+    resolution:
+      integrity: sha512-kdJUgym8a+vWzAwkqspFEXnDwD3QRWoNsak6H+CrhrrD3hcFsqeb3GiMUUZs5DY6zgMAsvgskfVia1DjxItW1Q==
   /currently-unhandled/0.4.1:
     dependencies:
       array-find-index: 1.0.2
@@ -1938,7 +1949,7 @@ packages:
   /delay/2.0.0:
     dependencies:
       p-defer: 1.0.0
-    dev: true
+    dev: false
     engines:
       node: '>=4'
     resolution:
@@ -2617,6 +2628,16 @@ packages:
     dev: false
     resolution:
       integrity: sha1-LANAXHU4w51+s3sxcCLjJfsBi/c=
+  /generate-function/1.1.0:
+    dev: false
+    resolution:
+      integrity: sha1-VMIbCAGSsW2Yd3ecW7gWZudyNl8=
+  /generate-object-property/1.2.0:
+    dependencies:
+      is-property: 1.0.2
+    dev: false
+    resolution:
+      integrity: sha1-nA4cQDCM6AT0eDYYuTf6iPmdUNA=
   /genfun/4.0.1:
     dev: false
     resolution:
@@ -2666,6 +2687,15 @@ packages:
       node: '>=0.12.0'
     resolution:
       integrity: sha1-Ei4WFZHiH/TFJTAwVpPyDmOTo5g=
+  /get-stream/2.3.1:
+    dependencies:
+      object-assign: 4.1.1
+      pinkie-promise: 2.0.1
+    dev: false
+    engines:
+      node: '>=0.10.0'
+    resolution:
+      integrity: sha1-Xzj5PzRgCWZu4BUKBUFn+Rvdld4=
   /get-stream/3.0.0:
     dev: false
     engines:
@@ -3078,6 +3108,14 @@ packages:
     dev: true
     resolution:
       integrity: sha1-dEi/qSQJKvMR1HFzu6uZDK4rsCc=
+  /into-stream/2.0.1:
+    dependencies:
+      from2: 2.3.0
+    dev: false
+    engines:
+      node: '>=0.10.0'
+    resolution:
+      integrity: sha1-25sANpRFPq4JHYpchMwRUHt4HTE=
   /into-stream/3.1.0:
     dependencies:
       from2: 2.3.0
@@ -3243,6 +3281,10 @@ packages:
     dev: true
     resolution:
       integrity: sha1-eaKp7OfwlugPNtKy87wWwf9L8/o=
+  /is-property/1.0.2:
+    dev: false
+    resolution:
+      integrity: sha1-V/4cTkhHTt1lsJkR8msc1Ald2oQ=
   /is-redirect/1.0.0:
     dev: false
     engines:
@@ -4078,6 +4120,16 @@ packages:
     dev: false
     resolution:
       integrity: sha1-rmA7NrE0vOw0e0UkIrC/mNWDLsg=
+  /neat-csv/2.1.0:
+    dependencies:
+      csv-parser: 1.12.0
+      get-stream: 2.3.1
+      into-stream: 2.0.1
+    dev: false
+    engines:
+      node: '>=4'
+    resolution:
+      integrity: sha1-BvWDYMTDuVW9Rn3cha5FEaOQekw=
   /negotiator/0.6.1:
     dev: true
     engines:
@@ -4602,7 +4654,7 @@ packages:
     resolution:
       integrity: sha512-RVbZPLso8+jFeq1MfNvgXtCRED2raz/dKpacfTNxsx6pLEpEomM7gah6VeHSYV3+vo0OAi4MkArtQcWWXuQoyw==
   /p-defer/1.0.0:
-    dev: true
+    dev: false
     engines:
       node: '>=4'
     resolution:
@@ -4961,13 +5013,11 @@ packages:
   /pinkie-promise/2.0.1:
     dependencies:
       pinkie: 2.0.4
-    dev: true
     engines:
       node: '>=0.10.0'
     resolution:
       integrity: sha1-ITXW36ejWMBprJsXh3YogihFD/o=
   /pinkie/2.0.4:
-    dev: true
     engines:
       node: '>=0.10.0'
     resolution:
@@ -5131,6 +5181,14 @@ packages:
     dev: false
     resolution:
       integrity: sha1-n/z7OsahVu4yt+vWnwJKT22JY1A=
+  /process-exists/3.0.0:
+    dependencies:
+      ps-list: 4.0.0
+    dev: false
+    engines:
+      node: '>=4'
+    resolution:
+      integrity: sha512-MK0GMLNYLqmUAR+9g4chQgQLqtE5U4Pvg8xwkvxe/EyAjmOniT0sJCs0UeS4B+6mQJpNMibzxmht99fo87fO6Q==
   /process-nextick-args/1.0.7:
     dev: false
     resolution:
@@ -5195,6 +5253,15 @@ packages:
     dev: false
     resolution:
       integrity: sha1-0/wRS6BplaRexok/SEzrHXj19HY=
+  /ps-list/4.0.0:
+    dependencies:
+      pify: 3.0.0
+      tasklist: 3.1.0
+    dev: false
+    engines:
+      node: '>=4'
+    resolution:
+      integrity: sha1-V8iz04Fh7ol3gRzTKl3FIjf9spk=
   /ps-tree/1.1.0:
     dependencies:
       event-stream: 3.3.4
@@ -5726,6 +5793,12 @@ packages:
     optional: true
     resolution:
       integrity: sha1-gaCY9Efku8P/MxKiQ1IbwGDvWRE=
+  /sec/1.0.0:
+    dev: false
+    engines:
+      node: '>=0.10.0'
+    resolution:
+      integrity: sha1-Az1go60g7PLgCUDRT5eCNGV3QzU=
   /semver-diff/2.1.0:
     dependencies:
       semver: 5.5.0
@@ -6359,6 +6432,16 @@ packages:
       node: '>=4.5'
     resolution:
       integrity: sha512-Ta5X6BSrA8QHznB156/nbqXUFf1M6A7rXrChKY5+6CkOjoFuOOBcJAj8FvMzDYyYBn7tb1UQNg4vUr7tkSlCUA==
+  /tasklist/3.1.0:
+    dependencies:
+      neat-csv: 2.1.0
+      pify: 2.3.0
+      sec: 1.0.0
+    dev: false
+    engines:
+      node: '>=4'
+    resolution:
+      integrity: sha1-hzqYpORcvez6LC7hiGU1MFfmNpY=
   /term-size/1.2.0:
     dependencies:
       execa: 0.7.0
@@ -6435,7 +6518,7 @@ packages:
     resolution:
       integrity: sha1-C2GKVWW23qkL80JdBNVe3EdadWE=
   /tree-kill/1.2.0:
-    dev: true
+    dev: false
     resolution:
       integrity: sha512-DlX6dR0lOIRDFxI0mjL9IYg6OTncLm/Zt+JiBhE5OlFcAR8yc9S7FFXU9so0oda47frdM/JFsk7UjNt9vscKcg==
   /trim-newlines/1.0.0:
@@ -7129,6 +7212,7 @@ specifiers:
   pnpm-file-reporter: ^0.0.1
   pnpm-list: ^2.0.0
   pnpm-registry-mock: ^1.5.0
+  process-exists: ^3.0.0
   ramda: ^0.25.0
   read-pkg: ^3.0.0
   retry: ^0.10.1
@@ -7142,11 +7226,11 @@ specifiers:
   tape: ^4.6.3
   tape-promise: ^2.0.1
   text-table: ^0.2.0
-  thenify: ^3.3.0
   tree-kill: ^1.2.0
   ts-node: ^4.0.0
   tslint: ^5.4.2
   typescript: ^2.4.1
   update-notifier: ^2.1.0
+  util.promisify: ^1.0.0
   write-json-file: ^2.3.0
   write-pkg: ^3.1.0

src/cmd/server/start.ts

@@ -44,7 +44,10 @@ export default async (
       : `http://${serverOptions.hostname}:${serverOptions.port}`,
   }
   const serverJsonPath = path.join(store.path, 'server.json')
-  await writeJsonFile(serverJsonPath, {connectionOptions})
+  await writeJsonFile(serverJsonPath, {
+    connectionOptions,
+    pid: process.pid,
+  })
 
   const server = createServer(store.ctrl, {
     ...serverOptions,

src/cmd/server/stop.ts

@@ -1,8 +1,14 @@
 import logger from '@pnpm/logger'
 import {connectStoreController} from '@pnpm/server'
+import delay = require('delay')
 import loadJsonFile = require('load-json-file')
 import {resolveStore} from 'package-store'
 import path = require('path')
+import processExists = require('process-exists')
+import killcb = require('tree-kill')
+import promisify = require('util.promisify')
+
+const kill = promisify(killcb)
 
 export default async (
   opts: {
@@ -11,13 +17,25 @@ export default async (
   },
 ) => {
   const store = await resolveStore(opts.store, opts.prefix)
+  let serverJson: any | undefined // tslint:disable-line
   try {
-    const serverJson = await loadJsonFile(path.join(store, 'server.json'))
-    const storeController = await connectStoreController(serverJson.connectionOptions)
-    await storeController.stop()
-    return
+    serverJson = await loadJsonFile(path.join(store, 'server.json'))
   } catch (err) {
-    if (err.code !== 'ENOENT') throw err
+    if (err.code !== 'ENOENT') {
+      throw err
+    } else {
+      logger.info(`Nothing to stop. No server is running for the store at ${store}`)
+      return
+    }
   }
-  logger.info(`Nothing to stop. No server is running for the store at ${store}`)
+  const storeController = await connectStoreController(serverJson.connectionOptions)
+  await storeController.stop()
+
+  if (!await processExists(serverJson.pid) || await delay(5000) && !await processExists(serverJson.pid)) {
+    logger.info('Server gracefully stopped')
+    return
+  }
+  logger.warn('Graceful shutdown failed')
+  await kill(serverJson.pid, 'SIGINT')
+  logger.info('Server process terminated')
 }

test/link.ts

@@ -9,7 +9,6 @@ import {
   execPnpm,
   isExecutable,
  } from './utils'
-import thenify = require('thenify')
 import fs = require('mz/fs')
 import isWindows = require('is-windows')
 

test/recursive.ts

@@ -4,7 +4,6 @@ import isCI = require('is-ci')
 import isWindows = require('is-windows')
 import tape = require('tape')
 import promisifyTape from 'tape-promise'
-import thenify = require('thenify')
 import path = require('path')
 import {
   prepare,

test/server.ts

@@ -6,7 +6,7 @@ import tape = require('tape')
 import promisifyTape from 'tape-promise'
 import killcb = require('tree-kill')
 import pathExists = require('path-exists')
-import thenify = require('thenify')
+import promisify = require('util.promisify')
 import {
   prepare,
   execPnpm,
@@ -17,7 +17,7 @@ import {
 
 const IS_WINDOWS = isWindows()
 const test = promisifyTape(tape)
-const kill = thenify(killcb)
+const kill = promisify(killcb)
 
 test('installation using pnpm server', async (t: tape.Test) => {
   const project = prepare(t)

test/uninstall.ts

@@ -7,7 +7,6 @@ import {
   testDefaults,
   execPnpm,
 } from './utils'
-import thenify = require('thenify')
 import path = require('path')
 import isWindows = require('is-windows')
 import exists = require('path-exists')

typings/local.d.ts

@@ -68,7 +68,7 @@ declare module 'rimraf-then' {
   export = anything;
 }
 
-declare module 'thenify' {
+declare module 'util.promisify' {
   const anything: any;
   export = anything;
 }
@@ -308,3 +308,8 @@ declare module 'diable' {
   const anything: any;
   export = anything;
 }
+
+declare module 'process-exists' {
+  const anything: any;
+  export = anything;
+}