Lars Volker has posted comments on this change. Change subject: Add Google Breakpad support to Kudu ......................................................................
Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/5620/3/src/kudu/util/minidump.cc File src/kudu/util/minidump.cc: PS3, Line 85: It logs the event before breakpad crashes : // the process > I hadn't considered this, but it turned out to be an issue. Yes, we had to do the same (https://github.com/cloudera/native-toolchain/commit/df2c2fa07c5a2394f8c6581c770201e19312f397). I didn't remember until I read this comment though. :( Line 118: minidump_exception_handler->WriteMinidump(FLAGS_minidump_path, DumpCallback, nullptr); > Hrmm. I was interpreting it the other way, but I think you're right... this Good catch! Looking at the implementation, WriteMinidump() calls memset(), which is not async-signal-safe. The Minidump itself will be written from a fork()'ed child, so I assume the window for the race is rather small. However, I'm not sure I completely understood the severity of this, so I will reach out to you in an email. I created IMPALA-4796 to track this. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
