Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
TBR=vabr@chromium.org (ICWYU)
TBR=ben@chromium.org (toplevel ICWYU)

Committed: https://crrev.com/cf1d7d0b6ef21635ebaaa217389f108a3d6db6e8
Cr-Commit-Position: refs/heads/master@{#433795}

[Ken Rockot(use gerrit already), 2016-11-18 22:53:06]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 22:53:33]

Dry run: 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-11-19 00:05:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-19 00:05:35]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-11-19 01:19:20]

Please take a look. Thanks!

[yzshen1, 2016-11-20 06:07:35]

https://codereview.chromium.org/2514093002/diff/1/mojo/edk/test/scoped_ipc_su...
File mojo/edk/test/scoped_ipc_support.cc (right):

https://codereview.chromium.org/2514093002/diff/1/mojo/edk/test/scoped_ipc_su...
mojo/edk/test/scoped_ipc_support.cc:39: quit_closure.Run();
(My understanding of the embedder API is purely from the comments. :))

I think this method is called from the IO thread, which is usually different
from the thread where the destructor of ScopedIPCSupport is called. If that is
the case, calling quit_closure.Run directly is not safe, right?

[Ken Rockot(use gerrit already), 2016-11-21 17:02:23]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 17:02:42]

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

[Ken Rockot(use gerrit already), 2016-11-21 17:02:59]

On 2016/11/20 at 06:07:35, yzshen wrote:
>
https://codereview.chromium.org/2514093002/diff/1/mojo/edk/test/scoped_ipc_su...
> File mojo/edk/test/scoped_ipc_support.cc (right):
> 
>
https://codereview.chromium.org/2514093002/diff/1/mojo/edk/test/scoped_ipc_su...
> mojo/edk/test/scoped_ipc_support.cc:39: quit_closure.Run();
> (My understanding of the embedder API is purely from the comments. :))
> 
> I think this method is called from the IO thread, which is usually different
from the thread where the destructor of ScopedIPCSupport is called. If that is
the case, calling quit_closure.Run directly is not safe, right?

You are correct. I've changed it to always PostTask to whatever
ThreadTaskRunnerHandle is active within the destructor.

[Ken Rockot(use gerrit already), 2016-11-21 17:04:00]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 17:04:16]

Dry run: 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-11-21 17:29:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 17:29:42]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yzshen1, 2016-11-21 17:37:57]

lgtm

[Ken Rockot(use gerrit already), 2016-11-21 17:41:00]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 17:41:34]

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

[Ken Rockot(use gerrit already), 2016-11-21 17:43:02]

Description was changed from

==========
Mojo EDK: Clean shutdown for test processes

This properly supports clean shutdown in EDK test processes,
allowing IO thread shutdown to be blocked on internal EDK
cleanup. Fixes some rare races in child process exit which can
cause message pipe breakage.

This does not fix all mojo_system_unittests flake, but it
addresses one of the two outstanding issues.

BUG=666356
R=yzshen@chromium.org
==========

to

==========
Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
==========

[Ken Rockot(use gerrit already), 2016-11-21 17:53:58]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-21 17:53:59]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-21 17:53:59]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2514093002/#ps60001
(title: ".")

[commit-bot: I haz the power, 2016-11-21 17:54:16]

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-11-21 18:44:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 18:44:34]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Rockot(use gerrit already), 2016-11-21 18:45:17]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-11-21 18:45:50]

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

[Ken Rockot(use gerrit already), 2016-11-21 18:46:21]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-21 20:46:07]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 20:46:50]

Dry run: 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-11-21 20:49:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 20:49:45]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Ken Rockot(use gerrit already), 2016-11-21 20:51:26]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 20:52:13]

Dry run: 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-11-21 21:01:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 21:01:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-11-21 21:08:32]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 21:09:11]

Dry run: 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-11-21 23:04:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 23:04:40]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ken Rockot(use gerrit already), 2016-11-21 23:57:28]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 23:58:08]

Dry run: 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-11-22 00:12:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:12:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Ken Rockot(use gerrit already), 2016-11-22 00:14:56]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 00:15:22]

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

[Ken Rockot(use gerrit already), 2016-11-22 00:24:12]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-22 00:24:14]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-22 00:24:14]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2514093002/#ps160001
(title: ".")

[commit-bot: I haz the power, 2016-11-22 00:24:47]

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-11-22 00:33:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:33:29]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Ken Rockot(use gerrit already), 2016-11-22 00:36:04]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 00:36:36]

Dry run: 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-11-22 00:55:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 00:55:40]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-11-22 01:01:19]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[Ken Rockot(use gerrit already), 2016-11-22 01:01:21]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-22 01:01:22]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2514093002/#ps200001
(title: "MORE ICWYU")

[Ken Rockot(use gerrit already), 2016-11-22 01:01:41]

Description was changed from

==========
Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
==========

to

==========
Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
TBR=vabr@chromium.org (ICWYU)
TBR=ben@chromium.org (toplevel ICWYU)
==========

[commit-bot: I haz the power, 2016-11-22 01:02:10]

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-11-22 01:20:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 01:20:29]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ken Rockot(use gerrit already), 2016-11-22 01:36:24]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-11-22 01:36:59]

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-11-22 02:03:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-22 02:03:21]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Ken Rockot(use gerrit already), 2016-11-22 04:09:45]

The CQ bit was checked by rockot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 04:10:02]

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

[Ken Rockot(use gerrit already), 2016-11-22 04:56:15]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-22 04:56:24]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-11-22 04:56:24]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2514093002/#ps220001
(title: "table flip emoji")

[commit-bot: I haz the power, 2016-11-22 04:56:43]

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-11-22 05:27:31]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1479790584603900,
"parent_rev": "45d3d67713d9a74ba07b4b435e0feac48b2157d3", "commit_rev":
"017621d6ec2a9c391aaca99944a4f5b0ebeaef38"}

[commit-bot: I haz the power, 2016-11-22 05:27:54]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-11-22 05:29:49]

Description was changed from

==========
Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
TBR=vabr@chromium.org (ICWYU)
TBR=ben@chromium.org (toplevel ICWYU)
==========

to

==========
Mojo EDK: Clean shutdown for ScopedIPCSupport in tests

This properly supports clean shutdown in test processes which
initialize the EDK, blocking ScopedIPCSupport destruction to wait
on EDK shutdown. Fixes some rare races in child process exit which
can cause premature message pipe breakage.

BUG=666356
R=yzshen@chromium.org
TBR=vabr@chromium.org (ICWYU)
TBR=ben@chromium.org (toplevel ICWYU)

Committed: https://crrev.com/cf1d7d0b6ef21635ebaaa217389f108a3d6db6e8
Cr-Commit-Position: refs/heads/master@{#433795}
==========

[commit-bot: I haz the power, 2016-11-22 05:29:50]

Patchset 12 (id:??) landed as
https://crrev.com/cf1d7d0b6ef21635ebaaa217389f108a3d6db6e8
Cr-Commit-Position: refs/heads/master@{#433795}