Fixed example in MessageLoop::ReleaseSoon comment

Fixed example in MessageLoop::ReleaseSoon comment

The previous version could cause race condition if for whatever reason ReleaseSoon finished releasing on another thread before we got to "foo = NULL"

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273879

[vkuzkokov, 2014-05-29 16:24:03]

The example had some issues.

[maniscalco, 2014-05-29 16:40:36]

Yikes, good catch!

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
base/message_loop/message_loop.h:211: // (AddRef), issue a ReleaseSoon, then
clear the pointer.  The reference count
We should also switch the order of ReleaseSoon and clear in the text so it
matches the corrected example.

[Mark Mentovai, 2014-05-29 20:42:10]

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
base/message_loop/message_loop.h:219: // foo = NULL;
If everything is RefCountedThreadSafe, why should it matter whether this happens
before or after the call to ReleaseSoon?

[maniscalco, 2014-05-29 22:50:31]

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
base/message_loop/message_loop.h:219: // foo = NULL;
On 2014/05/29 20:42:10, Mark Mentovai wrote:
> If everything is RefCountedThreadSafe, why should it matter whether this
happens
> before or after the call to ReleaseSoon?

Here's my understanding.  vkuzkokov, correct me if I'm wrong.

In the example code I committed, foo was not cleared until *after* ReleaseSoon()
was called.  Under the covers, ReleaseSoon() will post a task to message_loop. 
If the message_loop thread is "really fast", it could finish executing the
Release() *before* ReleaseSoon() has even returned.  If this happens, foo will
be the last reference and clearing it will destroy the object.

Destroying the Foo here would be bad because the reason for using ReleaseSoon()
in the first place was to ensure it gets destroyed in some other thread (in this
case, message_loop).

[Mark Mentovai, 2014-05-29 22:53:05]

LGTM. I was thinking about it from a “safety” perspective, and it should be
“safe” if your foo is a RefCountedThreadSafe. I totally ignored the semantic
perspective, where ReleaseSoon with a different thread’s message loop means that
you probably want the delete to not happen on the current thread.

[vkuzkokov, 2014-05-30 07:12:56]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-05-30 07:13:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/301943003/20001

[vkuzkokov, 2014-05-30 07:16:18]

The CQ bit was unchecked by vkuzkokov@chromium.org

[vkuzkokov, 2014-05-30 07:19:32]

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/301943003/diff/20001/base/message_loop/messag...
base/message_loop/message_loop.h:211: // (AddRef), issue a ReleaseSoon, then
clear the pointer.  The reference count
On 2014/05/29 16:40:36, maniscalco wrote:
> We should also switch the order of ReleaseSoon and clear in the text so it
> matches the corrected example.

Done.

[vkuzkokov, 2014-05-30 07:19:38]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-05-30 07:21:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/301943003/40001

[commit-bot: I haz the power, 2014-05-30 11:52:29]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-30 18:07:20]

Change committed as 273879