Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(32)

Issue 288063004: Always Ignore SIGPIPE for NaCl both in SFI mode and non SFI mode. (Closed)

Created:
5 years, 6 months ago by hidehiko
Modified:
5 years, 6 months ago
Reviewers:
Mark Seaborn
CC:
chromium-reviews, hamaji
Visibility:
Public.

Description

Always ignore SIGPIPE for NaCl both in SFI mode and non SFI mode. Currently, if there is an access to the pipe, whose other side is already closed, SIGPIPE will be raised in some platform such as linux. It is unexpected behavior, and causes some test failure. This is what trusted plugin does in content/app/content_main_runner.cc. TEST=Ran PPAPINaClPNaClNonSfiTest.Audio locally many times and the flakiness was gone. Also run trybots. BUG=371662 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271000

Patch Set 1 #

Total comments: 6

Patch Set 2 : Rebase #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -0 lines) Patch
M components/nacl/loader/nacl_helper_linux.cc View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
hidehiko
Hi Mark, could you take a look?
5 years, 6 months ago (2014-05-15 06:43:25 UTC) #1
Mark Seaborn
LGTM https://codereview.chromium.org/288063004/diff/1/components/nacl/loader/nacl_helper_linux.cc File components/nacl/loader/nacl_helper_linux.cc (right): https://codereview.chromium.org/288063004/diff/1/components/nacl/loader/nacl_helper_linux.cc#newcode94 components/nacl/loader/nacl_helper_linux.cc:94: // Always ignore SIGPIPE. We check the return ...
5 years, 6 months ago (2014-05-15 16:21:24 UTC) #2
hidehiko
Thank you for review. Submitting... https://codereview.chromium.org/288063004/diff/1/components/nacl/loader/nacl_helper_linux.cc File components/nacl/loader/nacl_helper_linux.cc (right): https://codereview.chromium.org/288063004/diff/1/components/nacl/loader/nacl_helper_linux.cc#newcode94 components/nacl/loader/nacl_helper_linux.cc:94: // Always ignore SIGPIPE. ...
5 years, 6 months ago (2014-05-16 08:21:02 UTC) #3
hidehiko
The CQ bit was checked by hidehiko@chromium.org
5 years, 6 months ago (2014-05-16 08:21:06 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/hidehiko@chromium.org/288063004/40001
5 years, 6 months ago (2014-05-16 08:21:24 UTC) #5
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
5 years, 6 months ago (2014-05-16 10:32:39 UTC) #6
commit-bot: I haz the power
5 years, 6 months ago (2014-05-16 12:22:55 UTC) #7
Message was sent while issue was closed.
Change committed as 271000

Powered by Google App Engine
This is Rietveld 408576698