Reland "Open MessagePumpLibevent's pipe with O_CLOEXEC"

Reland "Open MessagePumpLibevent's pipe with O_CLOEXEC"

This prevents the pipe's fds from being leaked into child processes.

TEST=git cl try
BUG=653930
Committed: https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8
Cr-Commit-Position: refs/heads/master@{#424030}

[Luis Héctor Chávez, 2016-10-07 22:14:33]

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

[commit-bot: I haz the power, 2016-10-07 22:15:14]

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

[Luis Héctor Chávez, 2016-10-07 22:16:07]

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

[Luis Héctor Chávez, 2016-10-07 22:16:07]

PTAAL

Turns out fcntl(... F_SETFD ...) is disallowed in NaCl. I'll just skip setting
the fd CLOEXEC there. PS1 is the revert as-is and PS2 is the fixed version.

[dcheng, 2016-10-07 22:31:18]

LGTM, but why is this disallowed in nonsfi nacl?

[Luis Héctor Chávez, 2016-10-07 22:36:40]

On 2016/10/07 22:31:18, dcheng wrote:
> LGTM, but why is this disallowed in nonsfi nacl?

I'm not really sure. Based on the code it should be allowed:
https://cs.chromium.org/chromium/src/native_client/src/nonsfi/linux/linux_sys...
Maybe the values of the flags have a different ABI, similar to the way NaCl
handles the O_* flags?

[dcheng, 2016-10-07 22:38:20]

On 2016/10/07 22:36:40, Luis Héctor Chávez wrote:
> On 2016/10/07 22:31:18, dcheng wrote:
> > LGTM, but why is this disallowed in nonsfi nacl?
> 
> I'm not really sure. Based on the code it should be allowed:
>
https://cs.chromium.org/chromium/src/native_client/src/nonsfi/linux/linux_sys...
> Maybe the values of the flags have a different ABI, similar to the way NaCl
> handles the O_* flags?

Weird, I would expect the ABI to match between the toolchain and the runtime
environment.
I'm OK with submitting this as-is, but can we file a followup bug to investigate
what's going on with NaCl people?

[Luis Héctor Chávez, 2016-10-07 23:49:40]

The CQ bit was checked by lhchavez@chromium.org

[Luis Héctor Chávez, 2016-10-07 23:49:42]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2398253004/#ps40001
(title: "More robust NaCl workaround")

[commit-bot: I haz the power, 2016-10-07 23:50:06]

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-08 00:51:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-08 00:54:27]

Description was changed from

==========
Reland "Open MessagePumpLibevent's pipe with O_CLOEXEC"

This prevents the pipe's fds from being leaked into child processes.

TEST=git cl try
BUG=653930
==========

to

==========
Reland "Open MessagePumpLibevent's pipe with O_CLOEXEC"

This prevents the pipe's fds from being leaked into child processes.

TEST=git cl try
BUG=653930

Committed: https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8
Cr-Commit-Position: refs/heads/master@{#424030}
==========

[commit-bot: I haz the power, 2016-10-08 00:54:28]

Patchset 3 (id:??) landed as
https://crrev.com/80c77bfd8614a09e83b618a819bef076508bdcf8
Cr-Commit-Position: refs/heads/master@{#424030}