Add comment about leaked FileDescriptorWatcher::Controller::Watcher.

Add comment about leaked FileDescriptorWatcher::Controller::Watcher.

If the MessageLoopForIO is deleted before Watcher::StartWatching() runs,
|watcher_| is leaked. If the MessageLoopForIO is deleted after
Watcher::StartWatching() runs but before the DeleteSoon task runs,
|watcher_| is deleted from Watcher::WillDestroyCurrentMessageLoop().

BUG=645114
Committed: https://crrev.com/20e8000df85972c5fd53d287753a189c45b9408c
Cr-Commit-Position: refs/heads/master@{#422320}

[fdoray, 2016-09-30 21:15:01]

fdoray@chromium.org changed reviewers:
+ dcheng@chromium.org

[fdoray, 2016-09-30 21:15:01]

PTAL

A leak occurs if WatchReadable/WatchWritable are called just before the
MessageLoopForIO associated with the current thread is destroyed (if
StartWatching doesn't have enough time to run).

With base::Thread, that never happens.
https://cs.chromium.org/chromium/src/base/files/file_descriptor_watcher_posix...
says that the MessageLoopForIO must exist when WatchReadable/WatchWritable are
called. That means that when WatchReadable/WatchWritable post the StartWatching
task, the MessageLoopForIO exists. A base::Thread never exits before all its
tasks have run
https://cs.chromium.org/chromium/src/base/threading/thread.cc?l=362 Conclusion:
The StartWatching task will always run.

With base::TaskScheduler, that never happens because the MessageLoopForIO
associated with every TaskScheduler worker thread is never deleted.

[dcheng, 2016-09-30 22:12:50]

LGTM.

Presumably, we don't consider this a problem because this only happens when
we're exiting?

[fdoray, 2016-10-01 14:49:52]

On 2016/09/30 22:12:50, dcheng wrote:
> LGTM.
> 
> Presumably, we don't consider this a problem because this only happens when
> we're exiting?

Exactly. And it isn't a problem when a MessageLoopForIO is managed by a base::
Thread (see previous comment)

[fdoray, 2016-10-01 14:49:57]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-10-01 14:50:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-01 15:37:51]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-01 15:39:57]

Description was changed from

==========
Add comment about leaked FileDescriptorWatcher::Controller::Watcher.

If the MessageLoopForIO is deleted before Watcher::StartWatching() runs,
|watcher_| is leaked. If the MessageLoopForIO is deleted after
Watcher::StartWatching() runs but before the DeleteSoon task runs,
|watcher_| is deleted from Watcher::WillDestroyCurrentMessageLoop().

BUG=645114
==========

to

==========
Add comment about leaked FileDescriptorWatcher::Controller::Watcher.

If the MessageLoopForIO is deleted before Watcher::StartWatching() runs,
|watcher_| is leaked. If the MessageLoopForIO is deleted after
Watcher::StartWatching() runs but before the DeleteSoon task runs,
|watcher_| is deleted from Watcher::WillDestroyCurrentMessageLoop().

BUG=645114

Committed: https://crrev.com/20e8000df85972c5fd53d287753a189c45b9408c
Cr-Commit-Position: refs/heads/master@{#422320}
==========

[commit-bot: I haz the power, 2016-10-01 15:39:58]

Patchset 1 (id:??) landed as
https://crrev.com/20e8000df85972c5fd53d287753a189c45b9408c
Cr-Commit-Position: refs/heads/master@{#422320}