Merge pull request #10518 from electron/fix-makesingleinstance

Fix app.makeSingleInstance hanging on posix systems

atom/browser/api/atom_api_app.cc

@@ -579,6 +579,12 @@ void App::OnFinishLaunching(const base::DictionaryValue& launch_info) {
   Emit("ready", launch_info);
 }
 
+void App::OnPreMainMessageLoopRun() {
+  if (process_singleton_) {
+    process_singleton_->OnBrowserReady();
+  }
+}
+
 void App::OnAccessibilitySupportChanged() {
   Emit("accessibility-support-changed", IsAccessibilitySupportEnabled());
 }

atom/browser/api/atom_api_app.h

@@ -94,6 +94,7 @@ class App : public AtomBrowserClient::Delegate,
   base::FilePath GetAppPath() const;
   void RenderProcessReady(content::RenderProcessHost* host);
   void RenderProcessDisconnected(base::ProcessId host_pid);
+  void PreMainMessageLoopRun();
 
  protected:
   explicit App(v8::Isolate* isolate);
@@ -112,6 +113,7 @@ class App : public AtomBrowserClient::Delegate,
   void OnLogin(LoginHandler* login_handler,
                const base::DictionaryValue& request_details) override;
   void OnAccessibilitySupportChanged() override;
+  void OnPreMainMessageLoopRun() override;
 #if defined(OS_MACOSX)
   void OnWillContinueUserActivity(
       bool* prevent_default,

atom/browser/atom_browser_main_parts.cc

@@ -4,6 +4,7 @@
 
 #include "atom/browser/atom_browser_main_parts.h"
 
+#include "atom/browser/api/atom_api_app.h"
 #include "atom/browser/api/trackable_object.h"
 #include "atom/browser/atom_access_token_store.h"
 #include "atom/browser/atom_browser_client.h"
@@ -183,6 +184,8 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
   std::unique_ptr<base::DictionaryValue> empty_info(new base::DictionaryValue);
   Browser::Get()->DidFinishLaunching(*empty_info);
 #endif
+
+  Browser::Get()->PreMainMessageLoopRun();
 }
 
 bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {

atom/browser/browser.cc

@@ -171,6 +171,12 @@ void Browser::RequestLogin(
     observer.OnLogin(login_handler, *(request_details.get()));
 }
 
+void Browser::PreMainMessageLoopRun() {
+  for (BrowserObserver& observer : observers_) {
+    observer.OnPreMainMessageLoopRun();
+  }
+}
+
 void Browser::NotifyAndShutdown() {
   if (is_shutdown_)
     return;

atom/browser/browser.h

@@ -224,6 +224,8 @@ class Browser : public WindowListObserver {
   void RequestLogin(LoginHandler* login_handler,
                     std::unique_ptr<base::DictionaryValue> request_details);
 
+  void PreMainMessageLoopRun();
+
   void AddObserver(BrowserObserver* obs) {
     observers_.AddObserver(obs);
   }

atom/browser/browser_observer.h

@@ -55,6 +55,9 @@ class BrowserObserver {
   // The browser's accessibility suppport has changed.
   virtual void OnAccessibilitySupportChanged() {}
 
+  // The app message loop is ready
+  virtual void OnPreMainMessageLoopRun() {}
+
 #if defined(OS_MACOSX)
   // The browser wants to report that an user activity will resume. (macOS only)
   virtual void OnWillContinueUserActivity(

chromium_src/chrome/browser/process_singleton.h

@@ -74,6 +74,8 @@ class ProcessSingleton : public base::NonThreadSafe {
   // TODO(brettw): Make the implementation of this method non-platform-specific
   // by making Linux re-use the Windows implementation.
   NotifyResult NotifyOtherProcessOrCreate();
+  void StartListeningOnSocket();
+  void OnBrowserReady();
 
   // Sets ourself up as the singleton instance.  Returns true on success.  If
   // false is returned, we are not the singleton instance and the caller must
@@ -173,6 +175,8 @@ class ProcessSingleton : public base::NonThreadSafe {
   // because it posts messages between threads.
   class LinuxWatcher;
   scoped_refptr<LinuxWatcher> watcher_;
+  int sock_;
+  bool listen_on_ready_ = false;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(ProcessSingleton);

chromium_src/chrome/browser/process_singleton_posix.cc

@@ -55,6 +55,7 @@
 
 #include <stddef.h>
 
+#include "atom/browser/browser.h"
 #include "atom/common/atom_command_line.h"
 
 #include "base/base_paths.h"
@@ -719,8 +720,7 @@ ProcessSingleton::ProcessSingleton(
     const base::FilePath& user_data_dir,
     const NotificationCallback& notification_callback)
     : notification_callback_(notification_callback),
-      current_pid_(base::GetCurrentProcId()),
-      watcher_(new LinuxWatcher(this)) {
+      current_pid_(base::GetCurrentProcId()) {
   // The user_data_dir may have not been created yet.
   base::CreateDirectoryAndGetError(user_data_dir, nullptr);
 
@@ -881,6 +881,23 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcessOrCreate() {
       base::TimeDelta::FromSeconds(kTimeoutInSeconds));
 }
 
+void ProcessSingleton::StartListeningOnSocket() {
+  watcher_ = new LinuxWatcher(this);
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&ProcessSingleton::LinuxWatcher::StartListening,
+                 watcher_,
+                 sock_));
+}
+
+void ProcessSingleton::OnBrowserReady() {
+  if (listen_on_ready_) {
+    StartListeningOnSocket();
+    listen_on_ready_ = false;
+  }
+}
+
 ProcessSingleton::NotifyResult
 ProcessSingleton::NotifyOtherProcessWithTimeoutOrCreate(
     const base::CommandLine& command_line,
@@ -1031,13 +1048,13 @@ bool ProcessSingleton::Create() {
   if (listen(sock, 5) < 0)
     NOTREACHED() << "listen failed: " << base::safe_strerror(errno);
 
-  DCHECK(BrowserThread::IsMessageLoopValid(BrowserThread::IO));
-  BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
-      base::Bind(&ProcessSingleton::LinuxWatcher::StartListening,
-                 watcher_,
-                 sock));
+  sock_ = sock;
+
+  if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) {
+    StartListeningOnSocket();
+  } else {
+    listen_on_ready_ = true;
+  }
 
   return true;
 }

chromium_src/chrome/browser/process_singleton_win.cc

@@ -258,6 +258,9 @@ ProcessSingleton::NotifyOtherProcessOrCreate() {
   return result;
 }
 
+void ProcessSingleton::StartListeningOnSocket() {}
+void ProcessSingleton::OnBrowserReady() {}
+
 // Look for a Chrome instance that uses the same profile directory. If there
 // isn't one, create a message window with its title set to the profile
 // directory path.

spec/api-app-spec.js

@@ -157,6 +157,28 @@ describe('app module', function () {
     })
   })
 
+  describe('app.makeSingleInstance', function () {
+    it('prevents the second launch of app', function (done) {
+      this.timeout(120000)
+      const appPath = path.join(__dirname, 'fixtures', 'api', 'singleton')
+      // First launch should exit with 0.
+      let secondLaunched = false
+      const first = ChildProcess.spawn(remote.process.execPath, [appPath])
+      first.once('exit', (code) => {
+        assert.ok(secondLaunched)
+        assert.equal(code, 0)
+        done()
+      })
+      // Second launch should exit with 1.
+      const second = ChildProcess.spawn(remote.process.execPath, [appPath])
+      second.once('exit', (code) => {
+        assert.ok(!secondLaunched)
+        assert.equal(code, 1)
+        secondLaunched = true
+      })
+    })
+  })
+
   describe('app.relaunch', function () {
     let server = null
     const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-app-relaunch' : '/tmp/electron-app-relaunch'

spec/fixtures/api/singleton/main.js

@@ -0,0 +1,13 @@
+const {app} = require('electron')
+
+process.on('uncaughtException', () => {
+  app.exit(2)
+})
+
+const shouldExit = app.makeSingleInstance(() => {
+  process.nextTick(() => app.exit(0))
+})
+
+if (shouldExit) {
+  app.exit(1)
+}

spec/fixtures/api/singleton/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "electron-app-singleton",
+  "main": "main.js"
+}
+