From 9e90ea8734521f249d555256849b83710bb0dc1f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 24 Sep 2015 12:11:07 +0800 Subject: [PATCH 1/3] win: Fix leaking of fd when reading file in asar --- atom/common/asar/archive.cc | 20 +++++++++----------- atom/common/asar/archive.h | 1 + 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/atom/common/asar/archive.cc b/atom/common/asar/archive.cc index 0b2f59ae0f..969f958956 100644 --- a/atom/common/asar/archive.cc +++ b/atom/common/asar/archive.cc @@ -115,6 +115,14 @@ bool FillFileInfoWithNode(Archive::FileInfo* info, Archive::Archive(const base::FilePath& path) : path_(path), file_(path_, base::File::FLAG_OPEN | base::File::FLAG_READ), +#if defined(OS_WIN) + fd_(_open_osfhandle( + reinterpret_cast(file_.GetPlatformFile()), 0)), +#elif defined(OS_POSIX) + fd_(file_.GetPlatformFile()), +#else + fd_(-1), +#endif header_size_(0) { } @@ -271,17 +279,7 @@ bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) { } int Archive::GetFD() const { - if (!file_.IsValid()) - return -1; - -#if defined(OS_WIN) - return - _open_osfhandle(reinterpret_cast(file_.GetPlatformFile()), 0); -#elif defined(OS_POSIX) - return file_.GetPlatformFile(); -#else - return -1; -#endif + return fd_; } } // namespace asar diff --git a/atom/common/asar/archive.h b/atom/common/asar/archive.h index dda7aa78e0..f2ff2f76d6 100644 --- a/atom/common/asar/archive.h +++ b/atom/common/asar/archive.h @@ -69,6 +69,7 @@ class Archive { private: base::FilePath path_; base::File file_; + int fd_; uint32 header_size_; scoped_ptr header_; From 269f70c12ab8eaeff2da8e0f32ebcfffcb794d8f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 24 Sep 2015 12:15:18 +0800 Subject: [PATCH 2/3] spec: Reading asar file should not leak fd --- spec/asar-spec.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/asar-spec.coffee b/spec/asar-spec.coffee index 4ef9337f3a..1e6ee69103 100644 --- a/spec/asar-spec.coffee +++ b/spec/asar-spec.coffee @@ -8,6 +8,10 @@ describe 'asar package', -> describe 'node api', -> describe 'fs.readFileSync', -> + it 'does not leak fd', -> + for i in [1..10000] + fs.readFileSync(path.join(process.resourcesPath, 'atom.asar', 'renderer', 'api', 'lib', 'ipc.js')) + it 'reads a normal file', -> file1 = path.join fixtures, 'asar', 'a.asar', 'file1' assert.equal fs.readFileSync(file1).toString().trim(), 'file1' From 576257470bf1fe4ab7b68887a0526144f3b2f260 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 24 Sep 2015 12:20:29 +0800 Subject: [PATCH 3/3] spec: Remove the will-navigate test It is unreliable to test in renderer process, remove it for now. --- spec/api-browser-window-spec.coffee | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/api-browser-window-spec.coffee b/spec/api-browser-window-spec.coffee index de713a6545..2e4a66b92b 100644 --- a/spec/api-browser-window-spec.coffee +++ b/spec/api-browser-window-spec.coffee @@ -294,16 +294,6 @@ describe 'browser-window module', -> w.show() w.minimize() - describe 'will-navigate event', -> - @timeout 10000 - it 'emits when user starts a navigation', (done) -> - url = "file://#{fixtures}/pages/will-navigate.html" - w.webContents.on 'will-navigate', (event, u) -> - event.preventDefault() - assert.equal u, url - done() - w.loadUrl url - xdescribe 'beginFrameSubscription method', -> it 'subscribes frame updates', (done) -> w.loadUrl "file://#{fixtures}/api/blank.html"