Use scoped_refptr and RefCountedThreadSafe for TargetPolicy.

sandbox/win/src/broker_services.cc

 // Copyright (c) 2012 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 "sandbox/win/src/broker_services.h"
 
 #include <AclAPI.h>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
 #include "base/threading/platform_thread.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/scoped_process_information.h"
 #include "base/win/startup_information.h"
 #include "base/win/windows_version.h"
 #include "sandbox/win/src/app_container.h"
 #include "sandbox/win/src/process_mitigations.h"
 #include "sandbox/win/src/sandbox.h"
@@ skipping 29 matching lines @@
 enum {
   THREAD_CTRL_NONE,
   THREAD_CTRL_REMOVE_PEER,
   THREAD_CTRL_QUIT,
   THREAD_CTRL_LAST,
 };
 
 // Helper structure that allows the Broker to associate a job notification
 // with a job object and with a policy.
 struct JobTracker {
-  JobTracker(base::win::ScopedHandle job, sandbox::TargetPolicy* policy)
-      : job(job.Pass()), policy(policy) {
-  }
-  ~JobTracker() {
-    FreeResources();
-  }
+  JobTracker(base::win::ScopedHandle job,
+             scoped_refptr<sandbox::TargetPolicy> policy)
+      : job(job.Pass()), policy(policy.Pass()) {}
+  ~JobTracker() { FreeResources(); }
 
   // Releases the Job and notifies the associated Policy object to release its
   // resources as well.
   void FreeResources();
 
   base::win::ScopedHandle job;
-  sandbox::TargetPolicy* policy;
+  scoped_refptr<sandbox::TargetPolicy> policy;
 };
 
 void JobTracker::FreeResources() {
   if (policy) {
     BOOL res = ::TerminateJobObject(job.Get(), sandbox::SBOX_ALL_OK);
     DCHECK(res);
     // Closing the job causes the target process to be destroyed so this needs
     // to happen before calling OnJobEmpty().
     HANDLE stale_job_handle = job.Get();
     job.Close();
 
     // In OnJobEmpty() we don't actually use the job handle directly.
     policy->OnJobEmpty(stale_job_handle);
-    policy->Release();
     policy = NULL;
   }
 }
 
 // Helper structure that allows the broker to track peer processes
 struct PeerTracker {
   PeerTracker(DWORD process_id, HANDLE broker_job_port)
       : wait_object(NULL), id(process_id), job_port(broker_job_port) {
   }
 
@@ skipping 69 matching lines @@
 
   // Cancel the wait events and delete remaining peer trackers.
   for (PeerTrackerMap::iterator it = peer_map_.begin();
        it != peer_map_.end(); ++it) {
     DeregisterPeerTracker(it->second);
   }
 
   ::DeleteCriticalSection(&lock_);
 }
 
-TargetPolicy* BrokerServicesBase::CreatePolicy() {
-  return new TargetPolicy;
+scoped_refptr<TargetPolicy> BrokerServicesBase::CreatePolicy() {
+  return make_scoped_refptr(new TargetPolicy);
 }
 
 // The worker thread stays in a loop waiting for asynchronous notifications
 // from the job objects. Right now we only care about knowing when the last
 // process on a job terminates, but in general this is the place to tell
 // the policy about events.
 DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) {
   if (NULL == param)
     return 1;
 
@@ skipping 23 matching lines @@
       // that jobs can send and some of them depend on the job attributes set.
       JobTracker* tracker = reinterpret_cast<JobTracker*>(key);
 
       switch (events) {
         case JOB_OBJECT_MSG_ACTIVE_PROCESS_ZERO: {
           // The job object has signaled that the last process associated
           // with it has terminated. Assuming there is no way for a process
           // to appear out of thin air in this job, it safe to assume that
           // we can tell the policy to destroy the target object, and for
           // us to release our reference to the policy object.
+          // TODO(rickyz): Is tracker removed from tracker_list_ or destroyed?
           tracker->FreeResources();
           break;
         }
 
         case JOB_OBJECT_MSG_NEW_PROCESS: {
           ++target_counter;
           if (1 == target_counter) {
             ::ResetEvent(no_targets);
           }
           break;
@@ skipping 47 matching lines @@
   }
 
   NOTREACHED();
   return 0;
 }
 
 // SpawnTarget does all the interesting sandbox setup and creates the target
 // process inside the sandbox.
 ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
                                            const wchar_t* command_line,
-                                           TargetPolicy* policy,
+                                           scoped_refptr<TargetPolicy> policy,
                                            PROCESS_INFORMATION* target_info) {
   if (!exe_path)
     return SBOX_ERROR_BAD_PARAMS;
 
   if (!policy)
     return SBOX_ERROR_BAD_PARAMS;
 
   // Even though the resources touched by SpawnTarget can be accessed in
   // multiple threads, the method itself cannot be called from more than
   // 1 thread. This is to protect the global variables used while setting up
@@ skipping 124 matching lines @@
   if (ERROR_SUCCESS != win_result) {
     SpawnCleanup(target, win_result);
     return SBOX_ERROR_CREATE_PROCESS;
   }
 
   // Now the policy is the owner of the target.
   if (!policy->AddTarget(target)) {
     return SpawnCleanup(target, 0);
   }
 
-  // We are going to keep a pointer to the policy because we'll call it when
-  // the job object generates notifications using the completion port.
-  policy->AddRef();
   if (job.IsValid()) {
-    scoped_ptr<JobTracker> tracker(new JobTracker(job.Pass(), policy));
+    scoped_ptr<JobTracker> tracker(new JobTracker(job.Pass(), policy.Pass()));
 
     // There is no obvious recovery after failure here. Previous version with
     // SpawnCleanup() caused deletion of TargetProcess twice. crbug.com/480639
     CHECK(AssociateCompletionPort(tracker->job.Get(), job_port_.Get(),
                                   tracker.get()));
 
     // Save the tracker because in cleanup we might need to force closing
     // the Jobs.
     tracker_list_.push_back(tracker.release());
     child_process_ids_.insert(process_info.process_id());
@@ skipping 85 matching lines @@
     return SBOX_ERROR_UNSUPPORTED;
 
   base::string16 name = LookupAppContainer(sid);
   if (name.empty())
     return SBOX_ERROR_INVALID_APP_CONTAINER;
 
   return DeleteAppContainer(sid);
 }
 
 }  // namespace sandbox