summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Gao2018-04-18 20:11:01 -0500
committerJosh Gao2018-04-19 16:33:18 -0500
commit48383c806af629bb755ce120ba30e8cb70eb5f23 (patch)
tree157ffe1ed460694d62347540829b931ab4602b3a
parent22dc27b9fa46b20aca4f5982979681a858a97284 (diff)
downloadplatform-system-core-48383c806af629bb755ce120ba30e8cb70eb5f23.tar.gz
platform-system-core-48383c806af629bb755ce120ba30e8cb70eb5f23.tar.xz
platform-system-core-48383c806af629bb755ce120ba30e8cb70eb5f23.zip
tombstoned: don't create tombstones for failed dumps.
Instead of creating tombstone FDs in place and passing them out to crash_dump directly, create them as O_TMPFILEs and link them into place when crash_dump reports success, to avoid creating empty tombstones in cases like an aborting thread racing with another thread that manages to cleanly exit_group before the dump finishes. Bug: http://b/77729983 Test: debuggerd_test Test: adb shell 'for x in `seq 0 50`; do crasher; done' Change-Id: I31ce4fd4a524abf8bde57152450209483d9d0ba9
-rw-r--r--debuggerd/tombstoned/tombstoned.cpp75
1 files changed, 47 insertions, 28 deletions
diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp
index 1bf8f14d0..5dffa5b3c 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -61,10 +61,10 @@ enum CrashStatus {
61struct Crash { 61struct Crash {
62 ~Crash() { event_free(crash_event); } 62 ~Crash() { event_free(crash_event); }
63 63
64 unique_fd crash_fd; 64 unique_fd crash_tombstone_fd;
65 unique_fd crash_socket_fd;
65 pid_t crash_pid; 66 pid_t crash_pid;
66 event* crash_event = nullptr; 67 event* crash_event = nullptr;
67 std::string crash_path;
68 68
69 DebuggerdDumpType crash_type; 69 DebuggerdDumpType crash_type;
70}; 70};
@@ -109,24 +109,27 @@ class CrashQueue {
109 return &queue; 109 return &queue;
110 } 110 }
111 111
112 std::pair<unique_fd, std::string> get_output() { 112 unique_fd get_output() {
113 unique_fd result; 113 unique_fd result(openat(dir_fd_, ".", O_WRONLY | O_APPEND | O_TMPFILE | O_CLOEXEC, 0640));
114 std::string file_name = StringPrintf("%s%02d", file_name_prefix_.c_str(), next_artifact_);
115
116 // Unlink and create the file, instead of using O_TRUNC, to avoid two processes
117 // interleaving their output in case we ever get into that situation.
118 if (unlinkat(dir_fd_, file_name.c_str(), 0) != 0 && errno != ENOENT) {
119 PLOG(FATAL) << "failed to unlink tombstone at " << dir_path_ << "/" << file_name;
120 }
121
122 result.reset(openat(dir_fd_, file_name.c_str(),
123 O_CREAT | O_EXCL | O_WRONLY | O_APPEND | O_CLOEXEC, 0640));
124 if (result == -1) { 114 if (result == -1) {
125 PLOG(FATAL) << "failed to create tombstone at " << dir_path_ << "/" << file_name; 115 // We might not have O_TMPFILE. Try creating and unlinking instead.
116 result.reset(
117 openat(dir_fd_, ".temporary", O_WRONLY | O_APPEND | O_CREAT | O_TRUNC | O_CLOEXEC, 0640));
118 if (result == -1) {
119 PLOG(FATAL) << "failed to create temporary tombstone in " << dir_path_;
120 }
121 if (unlinkat(dir_fd_, ".temporary", 0) != 0) {
122 PLOG(FATAL) << "failed to unlink temporary tombstone";
123 }
126 } 124 }
125 return result;
126 }
127 127
128 std::string get_next_artifact_path() {
129 std::string file_name =
130 StringPrintf("%s/%s%02d", dir_path_.c_str(), file_name_prefix_.c_str(), next_artifact_);
128 next_artifact_ = (next_artifact_ + 1) % max_artifacts_; 131 next_artifact_ = (next_artifact_ + 1) % max_artifacts_;
129 return {std::move(result), dir_path_ + "/" + file_name}; 132 return file_name;
130 } 133 }
131 134
132 bool maybe_enqueue_crash(Crash* crash) { 135 bool maybe_enqueue_crash(Crash* crash) {
@@ -203,14 +206,17 @@ static void crash_completed_cb(evutil_socket_t sockfd, short ev, void* arg);
203 206
204static void perform_request(Crash* crash) { 207static void perform_request(Crash* crash) {
205 unique_fd output_fd; 208 unique_fd output_fd;
206 if (!intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd)) { 209 bool intercepted =
207 std::tie(output_fd, crash->crash_path) = CrashQueue::for_crash(crash)->get_output(); 210 intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd);
211 if (!intercepted) {
212 output_fd = CrashQueue::for_crash(crash)->get_output();
213 crash->crash_tombstone_fd.reset(dup(output_fd.get()));
208 } 214 }
209 215
210 TombstonedCrashPacket response = { 216 TombstonedCrashPacket response = {
211 .packet_type = CrashPacketType::kPerformDump 217 .packet_type = CrashPacketType::kPerformDump
212 }; 218 };
213 ssize_t rc = send_fd(crash->crash_fd, &response, sizeof(response), std::move(output_fd)); 219 ssize_t rc = send_fd(crash->crash_socket_fd, &response, sizeof(response), std::move(output_fd));
214 if (rc == -1) { 220 if (rc == -1) {
215 PLOG(WARNING) << "failed to send response to CrashRequest"; 221 PLOG(WARNING) << "failed to send response to CrashRequest";
216 goto fail; 222 goto fail;
@@ -222,7 +228,7 @@ static void perform_request(Crash* crash) {
222 struct timeval timeout = { 10, 0 }; 228 struct timeval timeout = { 10, 0 };
223 229
224 event_base* base = event_get_base(crash->crash_event); 230 event_base* base = event_get_base(crash->crash_event);
225 event_assign(crash->crash_event, base, crash->crash_fd, EV_TIMEOUT | EV_READ, 231 event_assign(crash->crash_event, base, crash->crash_socket_fd, EV_TIMEOUT | EV_READ,
226 crash_completed_cb, crash); 232 crash_completed_cb, crash);
227 event_add(crash->crash_event, &timeout); 233 event_add(crash->crash_event, &timeout);
228 } 234 }
@@ -243,7 +249,7 @@ static void crash_accept_cb(evconnlistener* listener, evutil_socket_t sockfd, so
243 // and only native crashes on the native socket. 249 // and only native crashes on the native socket.
244 struct timeval timeout = { 1, 0 }; 250 struct timeval timeout = { 1, 0 };
245 event* crash_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, crash_request_cb, crash); 251 event* crash_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, crash_request_cb, crash);
246 crash->crash_fd.reset(sockfd); 252 crash->crash_socket_fd.reset(sockfd);
247 crash->crash_event = crash_event; 253 crash->crash_event = crash_event;
248 event_add(crash_event, &timeout); 254 event_add(crash_event, &timeout);
249} 255}
@@ -342,14 +348,27 @@ static void crash_completed_cb(evutil_socket_t sockfd, short ev, void* arg) {
342 goto fail; 348 goto fail;
343 } 349 }
344 350
345 if (!crash->crash_path.empty()) { 351 if (crash->crash_tombstone_fd != -1) {
346 if (crash->crash_type == kDebuggerdJavaBacktrace) { 352 std::string fd_path = StringPrintf("/proc/self/fd/%d", crash->crash_tombstone_fd.get());
347 LOG(ERROR) << "Traces for pid " << crash->crash_pid << " written to: " << crash->crash_path; 353 std::string tombstone_path = CrashQueue::for_crash(crash)->get_next_artifact_path();
354 int rc = unlink(tombstone_path.c_str());
355 if (rc != 0) {
356 PLOG(ERROR) << "failed to unlink tombstone at " << tombstone_path;
357 goto fail;
358 }
359
360 rc = linkat(AT_FDCWD, fd_path.c_str(), AT_FDCWD, tombstone_path.c_str(), AT_SYMLINK_FOLLOW);
361 if (rc != 0) {
362 PLOG(ERROR) << "failed to link tombstone";
348 } else { 363 } else {
349 // NOTE: Several tools parse this log message to figure out where the 364 if (crash->crash_type == kDebuggerdJavaBacktrace) {
350 // tombstone associated with a given native crash was written. Any changes 365 LOG(ERROR) << "Traces for pid " << crash->crash_pid << " written to: " << tombstone_path;
351 // to this message must be carefully considered. 366 } else {
352 LOG(ERROR) << "Tombstone written to: " << crash->crash_path; 367 // NOTE: Several tools parse this log message to figure out where the
368 // tombstone associated with a given native crash was written. Any changes
369 // to this message must be carefully considered.
370 LOG(ERROR) << "Tombstone written to: " << tombstone_path;
371 }
353 } 372 }
354 } 373 }
355 374