Getting service process on Mac to handle having things moved/changed underneath it.

chrome/common/service_process_util_mac.mm

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/service_process_util_posix.h"
 
 #import <Foundation/Foundation.h>
 #include <launch.h>
+#include <syslog.h>

[Mark Mentovai, 2011/03/11 20:13:52]

Why?

 
 #include "base/command_line.h"
 #include "base/file_path.h"
+#include "base/file_util.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/path_service.h"
+#include "base/process_util.h"
 #include "base/scoped_nsobject.h"
+#include "base/stringprintf.h"
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/version.h"
 #include "chrome/common/child_process_host.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "third_party/GTM/Foundation/GTMServiceManagement.h"
 
 namespace {
 
+#define kServiceProcessSessionType "Background"
+
 NSString* GetServiceProcessLaunchDFileName() {
   NSString *bundle_id = [base::mac::MainAppBundle() bundleIdentifier];
   NSString *label = [bundle_id stringByAppendingPathExtension:@"plist"];
   return label;
 }
 
 NSString* GetServiceProcessLaunchDLabel() {
   NSString *bundle_id = [base::mac::MainAppBundle() bundleIdentifier];
   NSString *label = [bundle_id stringByAppendingString:@".service_process"];
   FilePath user_data_dir;
@@ skipping 13 matching lines @@
 NSString* GetServiceProcessLaunchDSocketEnvVar() {
   NSString *label = GetServiceProcessLaunchDLabel();
   NSString *env_var = [label stringByReplacingOccurrencesOfString:@"."
                                                        withString:@"_"];
   env_var = [env_var stringByAppendingString:@"_SOCKET"];
   env_var = [env_var uppercaseString];
   return env_var;
 }
 
 // Creates the path that it returns. Must be called on the FILE thread.
-NSURL* GetUserAgentPath() {
+NSURL* GetUserAgentURL() {
   NSArray* library_paths = NSSearchPathForDirectoriesInDomains(
       NSLibraryDirectory, NSUserDomainMask, true);
   DCHECK_EQ([library_paths count], 1U);
   NSString* library_path = [library_paths objectAtIndex:0];
   NSString* launch_agents_path =
       [library_path stringByAppendingPathComponent:@"LaunchAgents"];
 
   NSError* err;
   if (![[NSFileManager defaultManager] createDirectoryAtPath:launch_agents_path
                                  withIntermediateDirectories:YES
                                                   attributes:nil
                                                        error:&err]) {
-    LOG(ERROR) << "GetUserAgentPath: " << err;
+    LOG(ERROR) << "GetUserAgentURL: " << err;
   }
 
   NSString* plist_file_path =
       [launch_agents_path
            stringByAppendingPathComponent:GetServiceProcessLaunchDFileName()];
   return [NSURL fileURLWithPath:plist_file_path isDirectory:NO];
 }
 
-}
+class ExecPathComponentWatcherDelegate
+    : public base::FilePathComponentWatcher::Delegate {
+ public:
+  ExecPathComponentWatcherDelegate(CFDictionaryRef launchd_conf,
+                                   ServiceProcessState *process_state)
+      : launchd_conf_(launchd_conf), process_state_(process_state) { }
+  virtual ~ExecPathComponentWatcherDelegate();
+  virtual void OnFilePathComponentsChanged(
+      base::FilePathComponentWatcher* watcher,
+      const FilePath& old_path,
+      const FilePath& new_path) OVERRIDE;
+
+private:
+  base::mac::ScopedCFTypeRef<CFDictionaryRef> launchd_conf_;
+  ServiceProcessState* process_state_;
+};
+
+}  // namespace
 
 // Gets the name of the service process IPC channel.
 IPC::ChannelHandle GetServiceProcessChannel() {
   std::string socket_path;
   scoped_nsobject<NSDictionary> dictionary(
       base::mac::CFToNSCast(GTMCopyLaunchdExports()));
   NSString *ns_socket_path =
       [dictionary objectForKey:GetServiceProcessLaunchDSocketEnvVar()];
   if (ns_socket_path) {
     socket_path = base::SysNSStringToUTF8(ns_socket_path);

[Mark Mentovai, 2011/03/11 20:13:52]

Uh-oh, seems that the existing code isn’t using -[NSString
fileSystemRepresentation] where it should be. File a follow-up bug?

   }
   return IPC::ChannelHandle(socket_path);
 }
 
 bool ForceServiceProcessShutdown(const std::string& /* version */,
                                  base::ProcessId /* process_id */) {
   NSString* label = GetServiceProcessLaunchDLabel();
   CFErrorRef err = NULL;
   bool ret = GTMSMJobRemove(reinterpret_cast<CFStringRef>(label), &err);
   if (!ret) {
@@ skipping 54 matching lines @@
   if (!InitializeState()) {
     return false;
   }
   CFErrorRef err = NULL;
   state_->launchd_conf_.reset(GTMSMJobCheckIn(&err));
   if (!state_->launchd_conf_.get()) {
     LOG(ERROR) << "InitializePlatformState: " << err;
     CFRelease(err);
     return false;
   }
+
+
+  return true;
+}
+
+bool ServiceProcessState::SignalReadyPlatformSpecific() {

[sanjeevr, 2011/03/11 08:26:42]

It seems kind of cheating to use the SignalReady to do something that has
nothing to do with signalling ready. Perhaps call this Initialize() or something
like that?

+  CFStringRef exe_path = reinterpret_cast<CFStringRef>(CFDictionaryGetValue(
+      state_->launchd_conf_, CFSTR(LAUNCH_JOBKEY_PROGRAM)));
+  if (!exe_path) {
+    return false;
+  }
+  FilePath executable_path = FilePath(base::SysCFStringRefToUTF8(exe_path));

[Mark Mentovai, 2011/03/11 20:13:52]

This is wrong. Use CFStringGetFileSystemRepresentation or -[NSString
fileSystemRepresentation]. The NSString method is less cumbersome and looks like
it’d be easy to use here.

+  if (!state_->executable_watcher_.Watch(executable_path,
+      new ExecPathComponentWatcherDelegate(state_->launchd_conf_, this))) {
+    return false;
+  }
   return true;
 }
 
 IPC::ChannelHandle ServiceProcessState::GetServiceProcessChannel() {
   CHECK(state_);
   NSDictionary *ns_launchd_conf = base::mac::CFToNSCast(state_->launchd_conf_);
   NSDictionary* socket_dict =
       [ns_launchd_conf objectForKey:@ LAUNCH_JOBKEY_SOCKETS];
   NSArray* sockets =
       [socket_dict objectForKey:GetServiceProcessLaunchDSocketKey()];
@@ skipping 80 matching lines @@
     // relaunch the service automatically in any other case than exiting
     // cleanly with a 0 return code.
     NSDictionary *keep_alive =
       [NSDictionary
         dictionaryWithObject:[NSNumber numberWithBool:NO]
                       forKey:@ LAUNCH_JOBKEY_KEEPALIVE_SUCCESSFULEXIT];
     NSDictionary *auto_launchd_plist =
       [[NSDictionary alloc] initWithObjectsAndKeys:
         [NSNumber numberWithBool:YES], @ LAUNCH_JOBKEY_RUNATLOAD,
         keep_alive, @ LAUNCH_JOBKEY_KEEPALIVE,
-        @"Background", @ LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE,
+        @ kServiceProcessSessionType, @ LAUNCH_JOBKEY_LIMITLOADTOSESSIONTYPE,
         nil];
     [launchd_plist addEntriesFromDictionary:auto_launchd_plist];
   }
   return reinterpret_cast<CFDictionaryRef>(launchd_plist);
 }
 
 // Writes the launchd property list into the user's LaunchAgents directory,
 // creating that directory if needed. This will cause the service process to be
 // auto launched on the next user login.
 bool ServiceProcessState::AddToAutoRun() {
   // We're creating directories and writing a file.
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(autorun_command_line_.get());
 
   base::mac::ScopedNSAutoreleasePool pool;
   scoped_nsobject<NSDictionary> plist(
       base::mac::CFToNSCast(CreateServiceProcessLaunchdPlist(
                                 autorun_command_line_.get(), true)));
-  NSURL* plist_url = GetUserAgentPath();
+  NSURL* plist_url = GetUserAgentURL();
   return [plist writeToURL:plist_url atomically:YES];
 }
 
 bool ServiceProcessState::RemoveFromAutoRun() {
   // We're killing a file.
   base::ThreadRestrictions::AssertIOAllowed();
 
   base::mac::ScopedNSAutoreleasePool pool;
-  NSURL* plist_url = GetUserAgentPath();
+  NSURL* plist_url = GetUserAgentURL();
   SInt32 error = 0;
   if (!CFURLDestroyResource(reinterpret_cast<CFURLRef>(plist_url), &error)) {
     LOG(ERROR) << "RemoveFromAutoRun: " << error;
     return false;
   }
   return true;
 }
+
+ExecPathComponentWatcherDelegate::~ExecPathComponentWatcherDelegate() {
+}
+
+void ExecPathComponentWatcherDelegate::OnFilePathComponentsChanged(
+    base::FilePathComponentWatcher* watcher, const FilePath& old_path,
+    const FilePath& new_path) {
+  base::mac::ScopedNSAutoreleasePool pool;
+  bool needs_shutdown = false;
+  bool needs_restart = false;
+  bool needs_plist_rewrite = false;
+  bool needs_plist_deletion = false;
+  if (access(new_path.value().c_str(), W_OK) != 0) {
+    // Can we execute it?

[Mark Mentovai, 2011/03/11 20:13:52]

Remember, no “we.” This is better:
    // Is it executable?

+    needs_shutdown = true;
+    needs_plist_deletion = true;
+  } else if (file_util::IsInTrash(new_path)) {
+    // Is it in the trash?
+    needs_shutdown = true;
+    needs_plist_deletion = true;
+  } else if (!file_util::AreReferringToSameObject(old_path, new_path)) {
+    // Did it get moved?
+    needs_restart = true;
+    needs_plist_rewrite = true;

[Mark Mentovai, 2011/03/11 20:13:52]

Judging from this logic, it seems that

  needs shutdown == needs_plist_deletion
and
  needs_restart == needs_plist_rewrite

True?

+  } else {
+    // Nothing exciting happened. Let's continue to ask for events.

[Mark Mentovai, 2011/03/11 20:13:52]

s/Let's c/C/

+    if (!watcher->Watch(new_path, this)) {

[Mark Mentovai, 2011/03/11 20:13:52]

OK, so it *is* possible to call Watch on an existing object, but only after it’s
called out?

?

Is it a better design (potentially losing fewer events) if the watcher is
allowed to keep watching until cancelled?

+      LOG(ERROR) << "Watch failed: " << new_path.value();
+    }
+  }
+  if (needs_plist_rewrite) {
+    NSURL* plist_url = GetUserAgentURL();
+    NSMutableDictionary* plist
+      = [NSMutableDictionary dictionaryWithContentsOfURL:plist_url];

[Mark Mentovai, 2011/03/11 20:13:52]

Put the = on the preceding line, and indent this one properly for a continuation
line.

+    if (plist) {
+      NSString* ns_new_path = base::SysUTF8ToNSString(new_path.value());

[Mark Mentovai, 2011/03/11 20:13:52]

Why not just +[NSString stringWithUTF8String:]?

+      [plist setObject:ns_new_path forKey:@ LAUNCH_JOBKEY_PROGRAM];
+      scoped_nsobject<NSMutableArray> args(
+          [[plist objectForKey:@ LAUNCH_JOBKEY_PROGRAMARGUMENTS] mutableCopy]);
+      [args replaceObjectAtIndex:0 withObject:ns_new_path];
+      [plist setObject:args forKey:@ LAUNCH_JOBKEY_PROGRAMARGUMENTS];
+      if (![plist writeToURL:plist_url atomically:YES]) {
+        LOG(ERROR) << "Unable to rewrite plist.";
+      }
+    } else {
+      LOG(ERROR) << "Unable to read plist.";
+    }
+  } else if (needs_plist_deletion) {
+    if (!process_state_->RemoveFromAutoRun()) {
+      LOG(ERROR) << "Unable to RemoveFromAutoRun.";
+    }
+  }
+  if (needs_shutdown || needs_restart) {
+    NSURL *plist_url = GetUserAgentURL();
+    std::string plist_path(base::SysNSStringToUTF8([plist_url path]));

[Mark Mentovai, 2011/03/11 20:13:52]

No, use [plist_url fileSystemRepresentation].

+    std::vector<std::string> argv;
+
+    if (needs_shutdown) {
+      VLOG(1) << "Shutting down using launchctl";
+      argv.push_back("/bin/launchctl");
+      argv.push_back("unload");
+      argv.push_back("-S");
+      argv.push_back(kServiceProcessSessionType);
+      argv.push_back(plist_path);
+    } else if (needs_restart) {

[Mark Mentovai, 2011/03/11 20:13:52]

This can just be |else|, look at the enclosing conditional.

+      VLOG(1) << "Restarting using launchctl";

[Mark Mentovai, 2011/03/11 20:13:52]

I assume that as soon as you run launchctl unload, this process will be killed?
Worth a comment.

I assume that’s also why you’re using a one-liner shell script to do this and
starting it in a new process group? Worth a comment.

+      argv.push_back("/bin/sh");

[Mark Mentovai, 2011/03/11 20:13:52]

/bin/bash if you’re using --noprofile.

+      argv.push_back("--noprofile");
+      argv.push_back("-c");
+      std::string command = base::StringPrintf(
+          "/bin/launchctl unload -S %s '%s';/bin/launchctl load -S %s '%s';",

[Mark Mentovai, 2011/03/11 20:13:52]

The only way I’ll let you pass arbitrary arguments to a shell script is if you
properly sanitize them. Using '%s' protects you against most nasties, but what
if plist_path winds up with a ' in it?

+           kServiceProcessSessionType, plist_path.c_str(),
+           kServiceProcessSessionType, plist_path.c_str());
+      argv.push_back(command);
+    }
+    base::ProcessHandle handle;
+    if (!base::LaunchAppInNewProcessGroup(argv,
+                                          base::environment_vector(),
+                                          base::file_handle_mapping_vector(),
+                                          NO,
+                                          &handle)) {
+      LOG(ERROR) << "LaunchAppInNewProcessGroup";
+    }

[Mark Mentovai, 2011/03/11 20:13:52]

What are you expecting to happen here? Be killed? What do you do? Return to the
run loop?

Do you have any cleanup that needs to be done?

Can you just exit?

Can you sleep forever expecting to be killed?

+  }
+}
+

[Mark Mentovai, 2011/03/11 20:13:52]

Blank line at EOF.