From cb0f636d407bee7b78fd21e050806f187e310137 Mon Sep 17 00:00:00 2001
From: Yu Lu <yu.lu@sky.uk>
Date: Wed, 15 Jun 2022 16:25:01 +0100
Subject: [PATCH] Fixing a potential `race condition` and possibly memory leak,
 where `powerShellStart` could be called multiple times, before
 `powerShellRelease` is called.

Imagine the following scenario:
1. function 1 calls `si.powerShellStart()` followed by `si.system(cb)` (or the promise version). It'll call "si.powerShellRelease" once it gets the response in the callback.
2. function 2 makes similar calls `si.powerShellStart()` followed by `si.os(cb)`.

There are a couple of issues here:

1. There's a chance that the call by function 2 could end up creating a new "PowerShell" process, which is not desirable, since creating "PowerShell" processes are expensive operation. It is also unnecessary to create the second "PowerShell" process, since function 2 should be able to just "reuse" the same "PowerShell" created by function 1.
2. The "PowerShell" process created by function 1 now becomes an "orphan", because `_psChild` is global therefore it's overwritten by function 2. The callback of function 1 may never be able to release it, where the "PowerShell" process created by function 2 are released twice.

This commit changes `powerShellStart` function so that it first check if a `persisted` "PowerShell" process already existed, and just reuse it if one is available.
---
 lib/util.js | 48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/lib/util.js b/lib/util.js
index 62320229..693ab9c5 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -396,6 +396,17 @@ function powerShellProceedResults(data) {
 }
 
 function powerShellStart() {
+  // Reuse existing persisted process, if one is available
+  if (_psPersistent && _psChild && _psChild.pid){
+    return;
+  }
+
+  // If somehow _psPersistent is not in sync with _psChild,
+  // just kill it and start a new one.
+  if (!_psPersistent && _psChild && _psChild.pid) {
+    powerShellRelease();
+  }
+
   _psChild = spawn('powershell.exe', ['-NoLogo', '-InputFormat', 'Text', '-NoExit', '-Command', '-'], {
     stdio: 'pipe',
     windowsHide: true,
@@ -403,25 +414,24 @@ function powerShellStart() {
     encoding: 'UTF-8',
     env: util._extend({}, process.env, { LANG: 'en_US.UTF-8' })
   });
-  if (_psChild && _psChild.pid) {
-    _psPersistent = true;
-    _psChild.stdout.on('data', function (data) {
-      _psResult = _psResult + data.toString('utf8');
-      if (data.indexOf(_psCmdSeperator) >= 0) {
-        powerShellProceedResults(_psResult);
-        _psResult = '';
-      }
-    });
-    _psChild.stderr.on('data', function () {
-      powerShellProceedResults(_psResult + _psError);
-    });
-    _psChild.on('error', function () {
-      powerShellProceedResults(_psResult + _psError);
-    });
-    _psChild.on('close', function () {
-      _psChild.kill();
-    });
-  }
+
+  _psPersistent = true;
+  _psChild.stdout.on('data', function (data) {
+    _psResult = _psResult + data.toString('utf8');
+    if (data.indexOf(_psCmdSeperator) >= 0) {
+      powerShellProceedResults(_psResult);
+      _psResult = '';
+    }
+  });
+  _psChild.stderr.on('data', function () {
+    powerShellProceedResults(_psResult + _psError);
+  });
+  _psChild.on('error', function () {
+    powerShellProceedResults(_psResult + _psError);
+  });
+  _psChild.on('close', function () {
+    _psChild.kill();
+  });
 }
 
 function powerShellRelease() {