diff options
author | Josh Gao | 2018-04-18 20:11:01 -0500 |
---|---|---|
committer | Josh Gao | 2018-04-19 16:33:18 -0500 |
commit | 48383c806af629bb755ce120ba30e8cb70eb5f23 (patch) | |
tree | 157ffe1ed460694d62347540829b931ab4602b3a | |
parent | 22dc27b9fa46b20aca4f5982979681a858a97284 (diff) | |
download | platform-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.cpp | 75 |
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 { | |||
61 | struct Crash { | 61 | struct 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 | ||
204 | static void perform_request(Crash* crash) { | 207 | static 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 | ||