New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 821254 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

IPCMojoBootstrapTest.ReceiveEmptyMessage test is inherently flaky

Project Member Reported by w...@chromium.org, Mar 13 2018

Issue description

The IPCMojoBootstrapTest.ReceiveEmptyMessage test is likely to flake, since it:

1. Launches a child process and SetPeerPid()s to it.
2. Has the child process also SetPeerPid(), back to the parent.
3. In both processes execute a QuitClosure() in response to SetPeerPid().
4. _after_ calling SetPeerPid() in the child process, calls Receive() in the parent.

The test is therefore dependent on the MessagePumpForIO seeing the I/O event from the Receive() message before it gets around to actually acting on the QuitClosure.

Introducing any delay between the SetPeerPid() and Receive() calls in the child process will cause the test to fail, now that it explicitly verifies that a message was received, if expected.  Previously the test would have passed despite the specified expectation not having been met.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4331bcefaa841a209f9cd3371ffd6e27f65247e

commit f4331bcefaa841a209f9cd3371ffd6e27f65247e
Author: Wez <wez@chromium.org>
Date: Tue Mar 13 03:12:12 2018

Disable IPCMojoBootstrapTest.ReceiveEmptyMessage temporarily.

This test is inherently flaky, so disable it pending a fix.

TBR: rockot
Bug:  821254 
Change-Id: Ic9db63f5a66d1e775033e6e2c9fa029beb7c5e42
Reviewed-on: https://chromium-review.googlesource.com/959746
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542710}
[modify] https://crrev.com/f4331bcefaa841a209f9cd3371ffd6e27f65247e/ipc/ipc_mojo_bootstrap_unittest.cc

Comment 2 by w...@chromium.org, Mar 13 2018

Owner: roc...@chromium.org
Status: Assigned (was: Started)
rockot: The broken ReceiveEmptyMessage test was added in December by https://chromium-review.googlesource.com/c/chromium/src/+/800076. The simplest fix is probably just to have both this and the Connect test set up to quit the RunLoop only when the child closes the underlying mojo::edk::Channel.

WDYT?
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/146d089c5cea00d18cfd772f7641acb0c036f699

commit 146d089c5cea00d18cfd772f7641acb0c036f699
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Mar 27 03:31:07 2018

Fix and re-enable IPCMojoBootstrapTest.ReceiveEmptyMessage

Lets the test run longer (i.e. until the child actually closes its
Channel) before asserting that we receive a message from the child.

Bug:  821254 
Change-Id: I629d994baf5655899dbd95177b997b97fd112e32
Reviewed-on: https://chromium-review.googlesource.com/981445
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545972}
[modify] https://crrev.com/146d089c5cea00d18cfd772f7641acb0c036f699/ipc/ipc_mojo_bootstrap_unittest.cc

Comment 4 by roc...@chromium.org, Mar 27 2018

Status: Fixed (was: Assigned)

Sign in to add a comment