You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/03/04 00:14:59 UTC
[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race
Hello David Ribeiro Alves,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/6259
to review the following change.
Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
......................................................................
subprocess: Get rid of fork/exec SIGPIPE reset race
There is a potential race in the current fork/exec procedure where,
after fork(), we set the signal disposition for SIGPIPE to SIG_DFL, and
then exec(). The problem with this sequence is that the parent process
may have outstanding threads performing socket or pipe writes which
could get interleaved between setting the signal handler to the default
and the exec().
The workaround in this patch is to set the signal handler in the child
to a no-op handler function in between fork() and exec(). Upon exec(),
the handler disposition is reset to SIG_DFL atomically and
automatically, resulting in the child getting SIG_DFL for SIGPIPE
without racy behavior.
Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
---
M src/kudu/util/subprocess.cc
1 file changed, 10 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/6259/1
--
To view, visit http://gerrit.cloudera.org:8080/6259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has abandoned this change.
Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/6259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
......................................................................
Patch Set 1:
Not sure I follow this. Between fork and exec there is only one thread running (fork only forks the thread that calls it).
--
To view, visit http://gerrit.cloudera.org:8080/6259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
......................................................................
Patch Set 1: Verified+1
Overriding unrelated failure due to KUDU-1736 (I attached the failure log to that JIRA)
--
To view, visit http://gerrit.cloudera.org:8080/6259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] subprocess: Get rid of fork/exec SIGPIPE reset race
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: subprocess: Get rid of fork/exec SIGPIPE reset race
......................................................................
Patch Set 1:
I thought fork included all threads. I guess the difference is that the child gets all the memory but only one of the threads. In that case, we don't need this patch after all. TFTR, I'll abandon.
--
To view, visit http://gerrit.cloudera.org:8080/6259
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ba5232775376fecbc05a14402a67d00e90818fa
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No