Jeffrey F. Lukman has posted comments on this change. ( http://gerrit.cloudera.org:8080/9015 )
Change subject: KUDU-2208 Add unit test to detect Subprocess Interruption Handling failure and patch to fix the bug ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/os-util.h File src/kudu/util/os-util.h: http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/os-util.h@33 PS5, Line 33: #define RETRY_ON_EINTR(err, expr) do { \ > need to #include <type_traits> in this header so that we get access to std: Done http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/subprocess-test.cc File src/kudu/util/subprocess-test.cc: http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/subprocess-test.cc@28 PS5, Line 28: #include <memory> > per Alexey's comment please separate out the C includes from the C++ ones h Done http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/subprocess-test.cc@312 PS5, Line 312: t_started = true; > still think we should add a short sleep here so that we ensure that the sig Done http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/subprocess-test.cc@313 PS5, Line 313: SleepFor(MonoDelta::FromMilliseconds(50)); > nit: we have to use CHECK_OK here because gtest isn't thread-safe to run as Done http://gerrit.cloudera.org:8080/#/c/9015/5/src/kudu/util/subprocess-test.cc@332 PS5, Line 332: ASSERT_TRUE(pthread_kill(t, SIGUSR2) == 0); > but the "user defined signal 2 error" is the bug we're trying to provoke he Done -- To view, visit http://gerrit.cloudera.org:8080/9015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I148b4619a8dda8e3e95dd6ea5c6e993a9e37a333 Gerrit-Change-Number: 9015 Gerrit-PatchSet: 6 Gerrit-Owner: Jeffrey F. Lukman <[email protected]> Gerrit-Reviewer: Jeffrey F. Lukman <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 12 Jan 2018 23:51:13 +0000 Gerrit-HasComments: Yes
