From bcb1630f20df969fc420249ce969c3c8397e5e5e Mon Sep 17 00:00:00 2001 From: Antoine Mercadal Date: Thu, 23 Apr 2015 16:46:01 -0700 Subject: [PATCH] FIXED: Closes all streams after a OS.popen() Previously, when popen was called, the 3 streams (stdin, stderr and stdourt) where never closed. This was the reason of the having an impossible number of open files. This patch ensure all streams are closed after using them. This means that the ulimit trick is no more necessary --- AppKit/Jakefile | 2 -- CommonJS/bin/press | 25 +++++++++++----- CommonJS/lib/cappuccino/fontinfo.j | 12 ++++++-- CommonJS/lib/cappuccino/imagesize.j | 12 ++++++-- Jakefile | 4 +++ Objective-J/CFHTTPRequest.js | 8 +++-- .../CommonJS/lib/objective-j/compiler.js | 11 ++++++- Objective-J/CommonJS/lib/objective-j/jake.js | 1 - Objective-J/Jakefile | 14 +++++---- Tools/XcodeCapp/Jakefile | 5 +++- Tools/nib2cib/Converter.j | 16 ++++++++-- Tools/nib2cib/main.j | 20 ------------- common.jake | 29 +++---------------- 13 files changed, 88 insertions(+), 71 deletions(-) diff --git a/AppKit/Jakefile b/AppKit/Jakefile index 70db16a3d..69f958e4c 100644 --- a/AppKit/Jakefile +++ b/AppKit/Jakefile @@ -1,8 +1,6 @@ require("../common.jake"); -checkUlimit(); - var framework = require("objective-j/jake").framework, BundleTask = require("objective-j/jake").BundleTask; diff --git a/CommonJS/bin/press b/CommonJS/bin/press index c4e6b5571..8d6c4c649 100755 --- a/CommonJS/bin/press +++ b/CommonJS/bin/press @@ -366,22 +366,31 @@ function pressEnvironment(rootPath, outputFiles, environment, options) { return {executable:includedBytes, data:dataBytes, mhtml:mhtmlBytes}; } -function pngcrushDirectory(directory) { - var directoryPath = FILE.path(directory); - var pngs = directoryPath.glob("**/*.png"); +function pngcrushDirectory(directory) +{ + var directoryPath = FILE.path(directory), + pngs = directoryPath.glob("**/*.png"); system.stderr.print("Running pngcrush on " + pngs.length + " pngs:"); - pngs.forEach(function(dstPath) { - var tmpPath = FILE.path(dstPath+".tmp"); + pngs.forEach(function(dstPath) + { + var tmpPath = FILE.path(dstPath+".tmp"), + p = OS.popen(["pngcrush", "-rem", "alla", "-reduce", /*"-brute",*/ dstPath, tmpPath]); - var p = OS.popen(["pngcrush", "-rem", "alla", "-reduce", /*"-brute",*/ dstPath, tmpPath]); - if (p.wait()) { + if (p.wait()) + { CPLog.warn("pngcrush failed. Ensure it's installed and on your PATH."); } - else { + else + { FILE.move(tmpPath, dstPath); system.stderr.write(".").flush(); } + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + }); system.stderr.print(""); } diff --git a/CommonJS/lib/cappuccino/fontinfo.j b/CommonJS/lib/cappuccino/fontinfo.j index beff36b9e..7fef16a5c 100644 --- a/CommonJS/lib/cappuccino/fontinfo.j +++ b/CommonJS/lib/cappuccino/fontinfo.j @@ -3,10 +3,18 @@ var OS = require("os"); exports.fontinfo = function(name, size) { - var p = OS.popen(["fontinfo", "-n", name, size || 12]); + var p = OS.popen(["fontinfo", "-n", name, size || 12]), + result; if (p.wait() === 0) - return JSON.parse(p.stdout.read()); + result = p.stdout.read(); + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + + if (result) + return JSON.parse(result); else return null; }; diff --git a/CommonJS/lib/cappuccino/imagesize.j b/CommonJS/lib/cappuccino/imagesize.j index c6e534bfa..af17c3cf3 100644 --- a/CommonJS/lib/cappuccino/imagesize.j +++ b/CommonJS/lib/cappuccino/imagesize.j @@ -3,10 +3,18 @@ var OS = require("os"); exports.imagesize = function(path) { - var p = OS.popen(["imagesize", "-n", path]); + var p = OS.popen(["imagesize", "-n", path]), + result; if (p.wait() === 0) - return JSON.parse(p.stdout.read()); + result = p.stdout.read(); + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + + if (result) + return JSON.parse(result); else return null; }; diff --git a/Jakefile b/Jakefile index 56a4e30aa..a03744328 100644 --- a/Jakefile +++ b/Jakefile @@ -135,6 +135,10 @@ function generateDocs(/* boolean */ noFrame) if (doxygenApps[0]) doxygen = FILE.join(doxygenApps[0], "Contents/Resources/doxygen"); } + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); } if (!doxygen || !FILE.exists(doxygen)) diff --git a/Objective-J/CFHTTPRequest.js b/Objective-J/CFHTTPRequest.js index 37bdf61e8..c2ccc4629 100644 --- a/Objective-J/CFHTTPRequest.js +++ b/Objective-J/CFHTTPRequest.js @@ -277,12 +277,12 @@ CFHTTPRequest.prototype.removeEventListener = function(/*String*/ anEventName, / this._eventDispatcher.removeEventListener(anEventName, anEventListener); }; -CFHTTPRequest.prototype.setWithCredentials = function(/*Boolean*/ willSendWithCredentials) +CFHTTPRequest.prototype.setWithCredentials = function(/*Boolean*/ willSendWithCredentials) { this._nativeRequest.withCredentials = willSendWithCredentials; }; -CFHTTPRequest.prototype.withCredentials = function() +CFHTTPRequest.prototype.withCredentials = function() { return this._nativeRequest.withCredentials; }; @@ -330,6 +330,10 @@ function FileRequest(/*CFURL*/ aURL, onsuccess, onfailure, onprogress) while (chunk = gcc.stdout.read()) fileContents += chunk; + gcc.stdin.close(); + gcc.stdout.close(); + gcc.stderr.close(); + if (fileContents.length > 0) { request._nativeRequest.responseText = fileContents; diff --git a/Objective-J/CommonJS/lib/objective-j/compiler.js b/Objective-J/CommonJS/lib/objective-j/compiler.js index 65079fd48..3bf18c5f2 100644 --- a/Objective-J/CommonJS/lib/objective-j/compiler.js +++ b/Objective-J/CommonJS/lib/objective-j/compiler.js @@ -46,7 +46,8 @@ function compileWithResolvedFlags(aFilePath, objjcFlags, gccFlags, asPlainJavasc if (!shouldObjjPreprocess) { - if (OS.popen("which gcc").stdout.read().length === 0) + var p = OS.popen("which gcc"); + if (p.stdout.read().length === 0) fileContents = FILE.read(aFilePath, { charset:"UTF-8" }); else { @@ -57,7 +58,15 @@ function compileWithResolvedFlags(aFilePath, objjcFlags, gccFlags, asPlainJavasc while (chunk = gcc.stdout.read()) fileContents += chunk; + gcc.stdin.close(); + gcc.stdout.close(); + gcc.stderr.close(); } + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + return fileContents; } diff --git a/Objective-J/CommonJS/lib/objective-j/jake.js b/Objective-J/CommonJS/lib/objective-j/jake.js index 7d4bc9048..b1683c037 100644 --- a/Objective-J/CommonJS/lib/objective-j/jake.js +++ b/Objective-J/CommonJS/lib/objective-j/jake.js @@ -22,7 +22,6 @@ var FILE = require("file"); - var BUNDLE_TASK = require("objective-j/jake/bundletask"); exports.BundleTask = BUNDLE_TASK.BundleTask; diff --git a/Objective-J/Jakefile b/Objective-J/Jakefile index ca5bfeda9..289cbacb0 100644 --- a/Objective-J/Jakefile +++ b/Objective-J/Jakefile @@ -40,8 +40,8 @@ $BROWSER_FILES = new FileList($BROWSER_FILE).include($OBJECTIVEJ_FILES); filedir($BUILD_BROWSER_FILE, $BROWSER_FILES, function(aTask) { - gcc($BROWSER_FILE, - $BUILD_BROWSER_FILE, + gcc($BROWSER_FILE, + $BUILD_BROWSER_FILE, environmentFlags("Browser", "ObjJ").concat($INCLUDE_FLAGS, $DEBUG_FLAGS), $CONFIGURATION !== "Debug"); }); @@ -119,12 +119,16 @@ function gcc(inputFilePath, outputFilePath, flags, compress) stream.print("Building... \0green(" + outputFilePath +"\0)"); // GCC preprocess the file. - var cmd = ["gcc", "-E", "-x", "c", "-P"].concat(flags, inputFilePath).join(" "); - var gcc = OS.popen(cmd, { charset:"UTF-8" }); + var cmd = ["gcc", "-E", "-x", "c", "-P"].concat(flags, inputFilePath).join(" "), + gcc = OS.popen(cmd, { charset:"UTF-8" }), + contents = FILE.read("header.txt", { charset : "UTF-8" }); - var contents = FILE.read("header.txt", { charset : "UTF-8" }); contents += gcc.stdout.read(); + gcc.stdin.close(); + gcc.stdout.close(); + gcc.stderr.close(); + if (FILE.extension(inputFilePath) === ".js" && compress) contents = compressor(contents); diff --git a/Tools/XcodeCapp/Jakefile b/Tools/XcodeCapp/Jakefile index 764cce92f..28245665b 100644 --- a/Tools/XcodeCapp/Jakefile +++ b/Tools/XcodeCapp/Jakefile @@ -25,11 +25,14 @@ task ("build", function() if (majorVersion < 10 || minorVersion < 7) { colorPrint("XcodeCapp can only be built on Mac OS X 10.7+. You can download the binary here: https://www.dropbox.com/sh/gxdgm356gyb9tqc/rFNyl8hcVG", "red"); - OS.exit(0); } } + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + if (executableExists("xcodebuild")) { var args = "-sdk macosx -alltargets -configuration Release", diff --git a/Tools/nib2cib/Converter.j b/Tools/nib2cib/Converter.j index 549a32bdf..58a2eedbf 100644 --- a/Tools/nib2cib/Converter.j +++ b/Tools/nib2cib/Converter.j @@ -119,8 +119,14 @@ ConverterConversionException = @"ConverterConversionException"; // Compile xib or nib to make sure we have a non-new format nib. temporaryNibFilePath = FILE.join("/tmp", FILE.basename(aFilePath) + ".tmp.nib"); - if (OS.popen(["/usr/bin/ibtool", aFilePath, "--compile", temporaryNibFilePath]).wait() === 1) + var p = OS.popen(["/usr/bin/ibtool", aFilePath, "--compile", temporaryNibFilePath]); + + if (p.wait() === 1) [CPException raise:ConverterConversionException reason:@"Could not compile file: " + aFilePath]; + + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); } else { @@ -130,9 +136,15 @@ ConverterConversionException = @"ConverterConversionException"; // Convert from binary plist to XML plist var temporaryPlistFilePath = FILE.join("/tmp", FILE.basename(aFilePath) + ".tmp.plist"); - if (OS.popen(["/usr/bin/plutil", "-convert", "xml1", temporaryNibFilePath, "-o", temporaryPlistFilePath]).wait() === 1) + var p = OS.popen(["/usr/bin/plutil", "-convert", "xml1", temporaryNibFilePath, "-o", temporaryPlistFilePath]); + + if (p.wait() === 1) [CPException raise:ConverterConversionException reason:@"Could not convert to xml plist for file: " + aFilePath]; + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + if (!FILE.isReadable(temporaryPlistFilePath)) [CPException raise:ConverterConversionException reason:@"Unable to convert nib file."]; diff --git a/Tools/nib2cib/main.j b/Tools/nib2cib/main.j index b5296c7b8..1bdc7aef1 100644 --- a/Tools/nib2cib/main.j +++ b/Tools/nib2cib/main.j @@ -28,27 +28,7 @@ var OS = require("os"), function main(args) { - checkUlimit(); - var nib2cib = [[Nib2Cib alloc] initWithArgs:args]; [nib2cib run]; } - -function checkUlimit() -{ - var minUlimit = 1024, - p = OS.popen(["ulimit", "-n"]); - - if (p.wait() === 0) - { - var limit = p.stdout.read().split("\n")[0]; - - if (Number(limit) < minUlimit) - { - stream.print("\0red(\0bold(WARNING:\0)\0) nib2cib may need to open more files than this terminal session currently allows (" + limit + "). Add the following line to your login configuration file (.bash_profile, .bashrc, etc.), start a new terminal session, then try again:\n"); - stream.print("ulimit -n " + minUlimit); - OS.exit(1); - } - } -} diff --git a/common.jake b/common.jake index f9a7f6569..103e658c4 100644 --- a/common.jake +++ b/common.jake @@ -385,6 +385,10 @@ global.setPackageMetadata = function(packagePath) pkg["cappuccino-revision"] = sha; } + p.stdin.close(); + p.stdout.close(); + p.stderr.close(); + pkg["cappuccino-timestamp"] = new Date().getTime(); pkg["version"] = getCappuccinoVersion(); @@ -577,31 +581,6 @@ global.colorPrint = function(/* String */ message, /* String */ color) stream.print(colorize(message, color)); }; -var minUlimit = 1024; - -global.checkUlimit = function() -{ - var ulimitPath = executableExists("ulimit"); - - if (!ulimitPath) - return; - - var p = OS.popen([ulimitPath, "-n"]); - - if (p.wait() === 0) - { - var limit = p.stdout.read().split("\n")[0]; - - if (Number(limit) < minUlimit) - { - stream.print("\0red(\0bold(ERROR:\0)\0) Cappuccino may need to open more files than this terminal session currently allows (" + limit + "). Add the following line to your login configuration file (.bash_profile, .bashrc, etc.), start a new terminal session, then try again:\n"); - stream.print("ulimit -n " + minUlimit); - OS.exit(1); - } - } -} - - // built in tasks task ("build");