Merge "Fix a few known issues in SensorService" into nyc-dev
diff --git a/cmds/dumpstate/bugreport-format.md b/cmds/dumpstate/bugreport-format.md
index 484f97f..d7837ae 100644
--- a/cmds/dumpstate/bugreport-format.md
+++ b/cmds/dumpstate/bugreport-format.md
@@ -26,16 +26,16 @@
is a failure, in which case it reverts to the flat file that is zipped by
**Shell** and hence the end result is the _v0_ format).
-The zip file is by default called _bugreport-DATE.zip_ and it contains a
-_bugreport-DATE.txt_ entry, although the end user can change the name (through
-**Shell**), in which case they would be called _bugreport-NEW_NAME.zip_ and
-_bugreport-NEW_NAME.txt_ respectively.
+The zip file is by default called _bugreport-BUILD_ID-DATE.zip_ and it contains a
+_bugreport-BUILD_ID-DATE.txt_ entry, although the end user can change the name (through
+**Shell**), in which case they would be called _bugreport-BUILD_ID-NEW_NAME.zip_ and
+_bugreport-BUILD_ID-NEW_NAME.txt_ respectively.
The zip file also contains 2 metadata entries generated by `dumpstate`:
- `version.txt`: whose value is **v1**.
- `main-entry.txt`: whose value is the name of the flat text entry (i.e.,
- _bugreport-DATE.txt_ or _bugreport-NEW_NAME.txt_).
+ _bugreport-BUILD_ID-DATE.txt_ or _bugreport-NEW_NAME.txt_).
`dumpstate` can also copy files from the device’s filesystem into the zip file
under the `FS` folder. For example, a `/dirA/dirB/fileC` file in the device
@@ -62,9 +62,9 @@
For example, the initial version during _Android N_ development was
**v1-dev1**. When `dumpsys` was split in 2 sections but not all tools were
-ready to parse that format, the version was named **v1-dev1-dumpsys-split**,
+ready to parse that format, the version was named **v1-dev2**,
which had to be passed do `dumpsys` explicitly (i.e., trhough a
-`-V v1-dev1-dumpsys-split` argument). Once that format became stable and tools
+`-V v1-dev2` argument). Once that format became stable and tools
knew how to parse it, the default version became **v1-dev2**.
Similarly, if changes in the file format are made after the initial release of
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 30ddec3..a368df8 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -92,6 +92,7 @@
*/
// TODO: change to "v1" before final N build
static std::string VERSION_DEFAULT = "v1-dev3";
+static std::string VERSION_BUILD_ON_NAME = "v1-dev4";
/* gets the tombstone data, according to the bugreport type: if zipped gets all tombstones,
* otherwise gets just those modified in the last half an hour. */
@@ -169,11 +170,15 @@
closedir(d);
}
-static void dump_systrace() {
+static void dump_systrace(const std::string& systrace_path) {
if (!zip_writer) {
MYLOGD("Not dumping systrace because zip_writer is not set\n");
return;
}
+ if (systrace_path.empty()) {
+ MYLOGE("Not dumping systrace because path is empty\n");
+ return;
+ }
const char* path = "/sys/kernel/debug/tracing/tracing_on";
long int is_tracing;
if (read_file_as_long(path, &is_tracing)) {
@@ -184,28 +189,24 @@
return;
}
- DurationReporter duration_reporter("SYSTRACE", nullptr);
- // systrace output can be many MBs, so we need to redirect its stdout straight to the zip file
- // by forking and using a pipe.
- int pipefd[2];
- pipe(pipefd);
- if (fork() == 0) {
- close(pipefd[0]); // close reading end in the child
- dup2(pipefd[1], STDOUT_FILENO); // send stdout to the pipe
- dup2(pipefd[1], STDERR_FILENO); // send stderr to the pipe
- close(pipefd[1]); // this descriptor is no longer needed
-
- // TODO: ideally it should use run_command, but it doesn't work well with pipes.
- // The drawback of calling execl directly is that we're not timing out if it hangs.
- MYLOGD("Running '/system/bin/atrace --async_dump', which can take several seconds");
- execl("/system/bin/atrace", "/system/bin/atrace", "--async_dump", nullptr);
- // execl should never return, but if it did, we need to exit.
- MYLOGD("execl on '/system/bin/atrace --async_dump' failed: %s", strerror(errno));
- // Must call _exit (instead of exit), otherwise it will corrupt the zip file.
- _exit(EXIT_FAILURE);
+ MYLOGD("Running '/system/bin/atrace --async_dump -o %s', which can take several minutes",
+ systrace_path.c_str());
+ if (run_command("SYSTRACE", 120, "/system/bin/atrace", "--async_dump", "-o",
+ systrace_path.c_str(), NULL)) {
+ MYLOGE("systrace timed out, its zip entry will be incomplete\n");
+ // TODO: run_command tries to kill the process, but atrace doesn't die peacefully; ideally,
+ // we should call strace to stop itself, but there is no such option yet (just a
+ // --async_stop, which stops and dump
+ // if (run_command("SYSTRACE", 10, "/system/bin/atrace", "--kill", NULL)) {
+ // MYLOGE("could not stop systrace ");
+ // }
+ }
+ if (!add_zip_entry("systrace.txt", systrace_path)) {
+ MYLOGE("Unable to add systrace file %s to zip file\n", systrace_path.c_str());
} else {
- close(pipefd[1]); // close the write end of the pipe in the parent
- add_zip_entry_from_fd("systrace.txt", pipefd[0]); // write output to zip file
+ if (remove(systrace_path.c_str())) {
+ MYLOGE("Error removing systrace file %s: %s", systrace_path.c_str(), strerror(errno));
+ }
}
}
@@ -939,8 +940,8 @@
" -B: send broadcast when finished (requires -o)\n"
" -P: send broadcast when started and update system properties on progress (requires -o and -B)\n"
" -R: take bugreport in remote mode (requires -o, -z, -d and -B, shouldn't be used with -P)\n"
- " -V: sets the bugreport format version (valid values: %s)\n",
- VERSION_DEFAULT.c_str());
+ " -V: sets the bugreport format version (valid values: %s, %s)\n",
+ VERSION_DEFAULT.c_str(), VERSION_BUILD_ON_NAME.c_str());
}
static void sigpipe_handler(int n) {
@@ -1092,7 +1093,7 @@
exit(1);
}
- if (version != VERSION_DEFAULT) {
+ if (version != VERSION_DEFAULT && version != VERSION_BUILD_ON_NAME) {
usage();
exit(1);
}
@@ -1116,6 +1117,9 @@
/* full path of the file containing the dumpstate logs*/
std::string log_path;
+ /* full path of the systrace file, when enabled */
+ std::string systrace_path;
+
/* full path of the temporary file containing the screenshot (when requested) */
std::string screenshot_path;
@@ -1145,6 +1149,11 @@
} else {
suffix = "undated";
}
+ if (version == VERSION_BUILD_ON_NAME) {
+ char build_id[PROPERTY_VALUE_MAX];
+ property_get("ro.build.id", build_id, "UNKNOWN_BUILD");
+ base_name = base_name + "-" + build_id;
+ }
if (do_fb) {
// TODO: if dumpstate was an object, the paths could be internal variables and then
// we could have a function to calculate the derived values, such as:
@@ -1154,6 +1163,7 @@
tmp_path = bugreport_dir + "/" + base_name + "-" + suffix + ".tmp";
log_path = bugreport_dir + "/dumpstate_log-" + suffix + "-"
+ std::to_string(getpid()) + ".txt";
+ systrace_path = bugreport_dir + "/systrace-" + suffix + ".txt";
MYLOGD("Bugreport dir: %s\n"
"Base name: %s\n"
@@ -1248,7 +1258,7 @@
print_header(version);
// Dumps systrace right away, otherwise it will be filled with unnecessary events.
- dump_systrace();
+ dump_systrace(systrace_path);
// Invoking the following dumpsys calls before dump_traces() to try and
// keep the system stats as close to its initial state as possible.
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index 8c0e840..5f9f24b 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -773,8 +773,8 @@
if (!waitpid_with_timeout(pid, 5, NULL)) {
kill(pid, SIGKILL);
if (!waitpid_with_timeout(pid, 5, NULL)) {
- printf("couldn not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
- MYLOGE("couldn not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
+ printf("could not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
+ MYLOGE("could not kill command '%s' (pid %d) even with SIGKILL.\n", command, pid);
}
}
return -1;