Allow IO operations in destructor of net::NetworkConfigWatcherMacThread

net/base/network_config_watcher_mac.cc

 // 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 "net/base/network_config_watcher_mac.h"
 
 #include <SystemConfiguration/SCDynamicStoreKey.h>
 #include <SystemConfiguration/SCSchemaDefinitions.h>
 #include <algorithm>
 
 #include "base/compiler_specific.h"
 #include "base/threading/thread.h"
 #include "base/mac/scoped_cftyperef.h"
+#include "base/threading/thread_restrictions.h"
 
 namespace net {
 
 namespace {
 
 // Called back by OS.  Calls OnNetworkConfigChange().
 void DynamicStoreCallback(SCDynamicStoreRef /* store */,
                           CFArrayRef changed_keys,
                           void* config_delegate) {
   NetworkConfigWatcherMac::Delegate* net_config_delegate =
@@ skipping 23 matching lines @@
   DISALLOW_COPY_AND_ASSIGN(NetworkConfigWatcherMacThread);
 };
 
 NetworkConfigWatcherMacThread::NetworkConfigWatcherMacThread(
     NetworkConfigWatcherMac::Delegate* delegate)
     : base::Thread("NetworkConfigWatcher"),
       delegate_(delegate),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {}
 
 NetworkConfigWatcherMacThread::~NetworkConfigWatcherMacThread() {
+  // Allow IO because Stop() calls PlatformThread::Join(), which is a blocking
+  // operation. This is expected during shutdown.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
   Stop();
 }
 
 void NetworkConfigWatcherMacThread::Init() {
+  // Disallow IO to make sure NetworkConfigWatcherMacThread's helper thread does
+  // not perform blocking operations.
+  base::ThreadRestrictions::SetIOAllowed(false);

[evanm, 2011/01/20 22:07:29]

Isn't this ok?  It's a helper thread, no?

[Raghu Simha, 2011/01/20 22:17:45]

Per email from willchan:

"Add a ScopedAllowIO in NetworkConfigWatcherMacThread's destructor. I don't
think that thread should be doing any blocking IO. In
NetworkConfigWatcherMacThread::Init(), we should probably do
base::ThreadRestrictions::SetIOAllowed(false), to make sure that
NetworkConfigWatcherMac's helper thread isn't blocking."

I'll defer to willchan's decision on whether this is necessary or not.

[willchan no longer on Chromium, 2011/01/20 22:47:10]

I think we should add it, because if we're going to join this thread in
non-shutdown situations, then we need to make sure the join is fast.

+
   // TODO(willchan): Look to see if there's a better signal for when it's ok to
   // initialize this, rather than just delaying it by a fixed time.
   const int kInitializationDelayMS = 1000;
   message_loop()->PostDelayedTask(
       FROM_HERE,
       method_factory_.NewRunnableMethod(
           &NetworkConfigWatcherMacThread::InitNotifications),
       kInitializationDelayMS);
 }
 
@@ skipping 33 matching lines @@
   // We create this notifier thread because the notification implementation
   // needs a thread with a CFRunLoop, and there's no guarantee that
   // MessageLoop::current() meets that criterion.
   base::Thread::Options thread_options(MessageLoop::TYPE_UI, 0);
   notifier_thread_->StartWithOptions(thread_options);
 }
 
 NetworkConfigWatcherMac::~NetworkConfigWatcherMac() {}
 
 }  // namespace net