Allow IO operations in destructor of net::NetworkConfigWatcherMacThread

Allow IO operations in destructor of net::NetworkConfigWatcherMacThread

The destructor of net::NetworkConfigWatcherMacThread ends up calling
PlatformThread::Join(), an operation that needs to be done only from an
IO thread. This currently causes a DCHECK to fire in base::ThreadRestrictions.

The bug was discovered by running the sync integration tests on a mac trybot.
The tests create Profile objects during setup, that indirectly call
~NetworkConfigWatcherMacThread() from their destructors.

This patch allows IO operations in ~NetworkConfigWatcherMacThread()
using a ScopedAllowIO.

BUG=70190
TEST=sync_integration_tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72030

[Raghu Simha, 2011-01-20 22:05:06]

+willchan for review.

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

I have no context, so I have stupid review feedback

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
File net/base/network_config_watcher_mac.cc (right):

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
net/base/network_config_watcher_mac.cc:68:
base::ThreadRestrictions::SetIOAllowed(false);
Isn't this ok?  It's a helper thread, no?

[Raghu Simha, 2011-01-20 22:17:41]

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
File net/base/network_config_watcher_mac.cc (right):

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
net/base/network_config_watcher_mac.cc:68:
base::ThreadRestrictions::SetIOAllowed(false);
On 2011/01/20 22:07:29, evanm wrote:
> Isn't this ok?  It's a helper thread, no?

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:09]

LGTM

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
File net/base/network_config_watcher_mac.cc (right):

http://codereview.chromium.org/6353008/diff/2001/net/base/network_config_watc...
net/base/network_config_watcher_mac.cc:68:
base::ThreadRestrictions::SetIOAllowed(false);
On 2011/01/20 22:17:45, rsimha wrote:
> On 2011/01/20 22:07:29, evanm wrote:
> > Isn't this ok?  It's a helper thread, no?
> 
> 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.

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.