jsreport/jsreport#202 avoid duplicated adding of process.args #10

pofider · Apr 7, 2016 · 9b25c89 · 9b25c89
1 parent 80d2219
commit 9b25c89
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 25 deletions.
diff --git a/index.js b/index.js
@@ -1,14 +1,22 @@
+function updateProcessArgs() {
+    //fix freeze during debugging
+    process.execArgv = _.filter(process.execArgv, function (arg) {
+        return !S(arg).startsWith("--debug");
+    });
+}
 
 module.exports = function(options) {
     var options = options || {};
     options.timeout = options.timeout || 10000;
     options.strategy = options.strategy || "http-server";
 
     if (options.strategy === "http-server") {
+        updateProcessArgs();
         return new (require("./lib/manager-servers.js"))(options);
     }
 
     if (options.strategy === "dedicated-process") {
+        updateProcessArgs();
         return new (require("./lib/manager-processes.js"))(options);
     }
 

diff --git a/lib/manager-processes.js b/lib/manager-processes.js
@@ -21,13 +21,6 @@ ScriptsManager.prototype.execute = function (inputs, options, cb) {
     var self = this;
     var isDone = false;
 
-    //fix freeze during debugging
-    process.execArgv = _.filter(process.execArgv, function (arg) {
-        return !S(arg).startsWith("--debug");
-    });
-
-    process.execArgv.push("--expose-gc");
-
     var worker = childProcess.fork(path.join(__dirname, "worker-processes.js"));
 
     worker.on('message', function (m) {

diff --git a/lib/manager-servers.js b/lib/manager-servers.js
@@ -48,6 +48,14 @@ var ScriptsManager = module.exports = function (options) {
     this.options.host = this.options.host || "127.0.0.1";
     this._runningRequests = [];
 
+    //there were some memory leaks in the old node.js versions when using vm module, the fix is to explicitly force gc
+    //here we expose gc to the forked node process
+    if (!_.filter(process.execArgv, function (arg) {
+          return !S(arg).startsWith("--expose-gc");
+      }).length) {
+        process.execArgv.push("--expose-gc");
+    }
+
     var self = this;
     process.once("exit", function () {
         self.kill();
@@ -75,13 +83,6 @@ ScriptsManager.prototype.start = function (cb) {
 
         self.options.port = port;
 
-        //fix freeze during debugging
-        process.execArgv = _.filter(process.execArgv, function (arg) {
-            return !S(arg).startsWith("--debug");
-        });
-
-        process.execArgv.push("--expose-gc");
-
         self.workersCluster = childProcess.fork(path.join(__dirname, "worker-servers.js"), []);
 
         self.workersCluster.on("exit", function() {

diff --git a/test/test.js b/test/test.js
@@ -4,7 +4,6 @@ var should = require("should"),
     ScriptsManagerWithProcesses = require("../lib/manager-processes.js");
     ScriptManagerInProcess = require("../lib/in-process.js");
 
-
 describe("scripts manager", function () {
 
     describe("servers", function () {
@@ -52,6 +51,16 @@ describe("scripts manager", function () {
                 done();
             });
         });
+
+        it("should expose gc", function (done) {
+            scriptsManager.execute({foo: "foo"}, {execModulePath: path.join(__dirname, "scripts", "gc.js")}, function (err, res) {
+                if (err)
+                    return done(err);
+
+                res.foo.should.be.eql("foo");
+                done();
+            });
+        });
     });
 
     describe("servers with custom settings", function (){
@@ -148,16 +157,6 @@ describe("scripts manager", function () {
                 done(new Error("There should be an error"));
             });
         });
-
-        it("should expose gc", function (done) {
-            scriptsManager.execute({foo: "foo"}, {execModulePath: path.join(__dirname, "scripts", "gc.js")}, function (err, res) {
-                if (err)
-                    return done(err);
-
-                res.foo.should.be.eql("foo");
-                done();
-            });
-        });
     }
 
     function common(scriptsManager) {