Merge "tombstoned: don't create tombstones for failed dumps."
am: 294d41cb83
Change-Id: Ia94317bd46773d6199853ded19ea804abbaf68c9
diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp
index 1bf8f14..5dffa5b 100644
--- a/debuggerd/tombstoned/tombstoned.cpp
+++ b/debuggerd/tombstoned/tombstoned.cpp
@@ -61,10 +61,10 @@
struct Crash {
~Crash() { event_free(crash_event); }
- unique_fd crash_fd;
+ unique_fd crash_tombstone_fd;
+ unique_fd crash_socket_fd;
pid_t crash_pid;
event* crash_event = nullptr;
- std::string crash_path;
DebuggerdDumpType crash_type;
};
@@ -109,24 +109,27 @@
return &queue;
}
- std::pair<unique_fd, std::string> get_output() {
- unique_fd result;
- std::string file_name = StringPrintf("%s%02d", file_name_prefix_.c_str(), next_artifact_);
-
- // Unlink and create the file, instead of using O_TRUNC, to avoid two processes
- // interleaving their output in case we ever get into that situation.
- if (unlinkat(dir_fd_, file_name.c_str(), 0) != 0 && errno != ENOENT) {
- PLOG(FATAL) << "failed to unlink tombstone at " << dir_path_ << "/" << file_name;
- }
-
- result.reset(openat(dir_fd_, file_name.c_str(),
- O_CREAT | O_EXCL | O_WRONLY | O_APPEND | O_CLOEXEC, 0640));
+ unique_fd get_output() {
+ unique_fd result(openat(dir_fd_, ".", O_WRONLY | O_APPEND | O_TMPFILE | O_CLOEXEC, 0640));
if (result == -1) {
- PLOG(FATAL) << "failed to create tombstone at " << dir_path_ << "/" << file_name;
+ // We might not have O_TMPFILE. Try creating and unlinking instead.
+ result.reset(
+ openat(dir_fd_, ".temporary", O_WRONLY | O_APPEND | O_CREAT | O_TRUNC | O_CLOEXEC, 0640));
+ if (result == -1) {
+ PLOG(FATAL) << "failed to create temporary tombstone in " << dir_path_;
+ }
+ if (unlinkat(dir_fd_, ".temporary", 0) != 0) {
+ PLOG(FATAL) << "failed to unlink temporary tombstone";
+ }
}
+ return result;
+ }
+ std::string get_next_artifact_path() {
+ std::string file_name =
+ StringPrintf("%s/%s%02d", dir_path_.c_str(), file_name_prefix_.c_str(), next_artifact_);
next_artifact_ = (next_artifact_ + 1) % max_artifacts_;
- return {std::move(result), dir_path_ + "/" + file_name};
+ return file_name;
}
bool maybe_enqueue_crash(Crash* crash) {
@@ -203,14 +206,17 @@
static void perform_request(Crash* crash) {
unique_fd output_fd;
- if (!intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd)) {
- std::tie(output_fd, crash->crash_path) = CrashQueue::for_crash(crash)->get_output();
+ bool intercepted =
+ intercept_manager->GetIntercept(crash->crash_pid, crash->crash_type, &output_fd);
+ if (!intercepted) {
+ output_fd = CrashQueue::for_crash(crash)->get_output();
+ crash->crash_tombstone_fd.reset(dup(output_fd.get()));
}
TombstonedCrashPacket response = {
.packet_type = CrashPacketType::kPerformDump
};
- ssize_t rc = send_fd(crash->crash_fd, &response, sizeof(response), std::move(output_fd));
+ ssize_t rc = send_fd(crash->crash_socket_fd, &response, sizeof(response), std::move(output_fd));
if (rc == -1) {
PLOG(WARNING) << "failed to send response to CrashRequest";
goto fail;
@@ -222,7 +228,7 @@
struct timeval timeout = { 10, 0 };
event_base* base = event_get_base(crash->crash_event);
- event_assign(crash->crash_event, base, crash->crash_fd, EV_TIMEOUT | EV_READ,
+ event_assign(crash->crash_event, base, crash->crash_socket_fd, EV_TIMEOUT | EV_READ,
crash_completed_cb, crash);
event_add(crash->crash_event, &timeout);
}
@@ -243,7 +249,7 @@
// and only native crashes on the native socket.
struct timeval timeout = { 1, 0 };
event* crash_event = event_new(base, sockfd, EV_TIMEOUT | EV_READ, crash_request_cb, crash);
- crash->crash_fd.reset(sockfd);
+ crash->crash_socket_fd.reset(sockfd);
crash->crash_event = crash_event;
event_add(crash_event, &timeout);
}
@@ -342,14 +348,27 @@
goto fail;
}
- if (!crash->crash_path.empty()) {
- if (crash->crash_type == kDebuggerdJavaBacktrace) {
- LOG(ERROR) << "Traces for pid " << crash->crash_pid << " written to: " << crash->crash_path;
+ if (crash->crash_tombstone_fd != -1) {
+ std::string fd_path = StringPrintf("/proc/self/fd/%d", crash->crash_tombstone_fd.get());
+ std::string tombstone_path = CrashQueue::for_crash(crash)->get_next_artifact_path();
+ int rc = unlink(tombstone_path.c_str());
+ if (rc != 0) {
+ PLOG(ERROR) << "failed to unlink tombstone at " << tombstone_path;
+ goto fail;
+ }
+
+ rc = linkat(AT_FDCWD, fd_path.c_str(), AT_FDCWD, tombstone_path.c_str(), AT_SYMLINK_FOLLOW);
+ if (rc != 0) {
+ PLOG(ERROR) << "failed to link tombstone";
} else {
- // NOTE: Several tools parse this log message to figure out where the
- // tombstone associated with a given native crash was written. Any changes
- // to this message must be carefully considered.
- LOG(ERROR) << "Tombstone written to: " << crash->crash_path;
+ if (crash->crash_type == kDebuggerdJavaBacktrace) {
+ LOG(ERROR) << "Traces for pid " << crash->crash_pid << " written to: " << tombstone_path;
+ } else {
+ // NOTE: Several tools parse this log message to figure out where the
+ // tombstone associated with a given native crash was written. Any changes
+ // to this message must be carefully considered.
+ LOG(ERROR) << "Tombstone written to: " << tombstone_path;
+ }
}
}