Merge "metricsd: move timer.h into include/metrics/"
diff --git a/adb/adb.cpp b/adb/adb.cpp
index b260a78..1249822 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -177,6 +177,7 @@
{"jdwp", TRACE_JDWP},
{"services", TRACE_SERVICES},
{"auth", TRACE_AUTH},
+ {"fdevent", TRACE_FDEVENT},
{"shell", TRACE_SHELL}};
std::vector<std::string> elements = android::base::Split(trace_setting, " ");
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index c508b32..d287480 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -524,6 +524,20 @@
return android::base::StringPrintf("%s:%s", prefix, command);
}
+// Checks whether the device indicated by |transport_type| and |serial| supports
+// |feature|. Returns the response string, which will be empty if the device
+// could not be found or the feature is not supported.
+static std::string CheckFeature(const std::string& feature,
+ TransportType transport_type,
+ const char* serial) {
+ std::string result, error, command("check-feature:" + feature);
+ if (!adb_query(format_host_command(command.c_str(), transport_type, serial),
+ &result, &error)) {
+ return "";
+ }
+ return result;
+}
+
static int adb_download_buffer(const char *service, const char *fn, const void* data, unsigned sz,
bool show_progress)
{
@@ -783,12 +797,15 @@
wait_for_device("wait-for-device", transport_type, serial);
}
- read_and_dump(fd);
- int rc = adb_close(fd);
- if (rc) {
- perror("close");
+ bool use_shell_protocol = !CheckFeature(kFeatureShell2, transport_type,
+ serial).empty();
+ int exit_code = read_and_dump(fd, use_shell_protocol);
+
+ if (adb_close(fd) < 0) {
+ PLOG(ERROR) << "failure closing FD " << fd;
}
- return rc;
+
+ return exit_code;
}
static int logcat(TransportType transport, const char* serial, int argc, const char** argv) {
@@ -1013,20 +1030,6 @@
#endif
}
-// Checks whether the device indicated by |transport_type| and |serial| supports
-// |feature|. Returns the response string, which will be empty if the device
-// could not be found or the feature is not supported.
-static std::string CheckFeature(const std::string& feature,
- TransportType transport_type,
- const char* serial) {
- std::string result, error, command("check-feature:" + feature);
- if (!adb_query(format_host_command(command.c_str(), transport_type, serial),
- &result, &error)) {
- return "";
- }
- return result;
-}
-
int adb_commandline(int argc, const char **argv) {
int no_daemon = 0;
int is_daemon = 0;
diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp
index db5157f..dc03807 100644
--- a/adb/fdevent.cpp
+++ b/adb/fdevent.cpp
@@ -20,59 +20,30 @@
#include "sysdeps.h"
#include "fdevent.h"
-#include <errno.h>
#include <fcntl.h>
-#include <stdarg.h>
-#include <stddef.h>
-#include <stdio.h>
+#include <poll.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>
+#include <list>
+#include <unordered_map>
+#include <vector>
+
+#include <base/logging.h>
+#include <base/stringprintf.h>
+
#include "adb_io.h"
#include "adb_trace.h"
-/* !!! Do not enable DEBUG for the adb that will run as the server:
-** both stdout and stderr are used to communicate between the client
-** and server. Any extra output will cause failures.
-*/
-#define DEBUG 0 /* non-0 will break adb server */
-
+#if !ADB_HOST
// This socket is used when a subproc shell service exists.
// It wakes up the fdevent_loop() and cause the correct handling
// of the shell's pseudo-tty master. I.e. force close it.
-#if !ADB_HOST
int SHELL_EXIT_NOTIFY_FD = -1;
#endif // !ADB_HOST
-static void fatal(const char *fn, const char *fmt, ...)
-{
- va_list ap;
- va_start(ap, fmt);
- fprintf(stderr, "%s:", fn);
- vfprintf(stderr, fmt, ap);
- va_end(ap);
- abort();
-}
-
-#define FATAL(x...) fatal(__FUNCTION__, x)
-
-#if DEBUG
-static void dump_fde(fdevent *fde, const char *info)
-{
- adb_mutex_lock(&D_lock);
- fprintf(stderr,"FDE #%03d %c%c%c %s\n", fde->fd,
- fde->state & FDE_READ ? 'R' : ' ',
- fde->state & FDE_WRITE ? 'W' : ' ',
- fde->state & FDE_ERROR ? 'E' : ' ',
- info);
- adb_mutex_unlock(&D_lock);
-}
-#else
-#define dump_fde(fde, info) do { } while(0)
-#endif
-
#define FDE_EVENTMASK 0x00ff
#define FDE_STATEMASK 0xff00
@@ -80,515 +51,47 @@
#define FDE_PENDING 0x0200
#define FDE_CREATED 0x0400
-static void fdevent_plist_enqueue(fdevent *node);
-static void fdevent_plist_remove(fdevent *node);
-static fdevent *fdevent_plist_dequeue(void);
+struct PollNode {
+ fdevent* fde;
+ pollfd pollfd;
-static fdevent list_pending = {
- .next = &list_pending,
- .prev = &list_pending,
- .fd = -1,
- .force_eof = 0,
- .state = 0,
- .events = 0,
- .func = nullptr,
- .arg = nullptr,
+ PollNode(fdevent* fde) : fde(fde) {
+ memset(&pollfd, 0, sizeof(pollfd));
+ pollfd.fd = fde->fd;
+ }
};
-static fdevent **fd_table = 0;
-static int fd_table_max = 0;
+// All operations to fdevent should happen only in the main thread.
+// That's why we don't need a lock for fdevent.
+static std::unordered_map<int, PollNode> g_poll_node_map;
+static std::list<fdevent*> g_pending_list;
-#ifdef CRAPTASTIC
-//HAVE_EPOLL
-
-#include <sys/epoll.h>
-
-static int epoll_fd = -1;
-
-static void fdevent_init()
-{
- /* XXX: what's a good size for the passed in hint? */
- epoll_fd = epoll_create(256);
-
- if(epoll_fd < 0) {
- perror("epoll_create() failed");
- exit(1);
+static std::string dump_fde(const fdevent* fde) {
+ std::string state;
+ if (fde->state & FDE_ACTIVE) {
+ state += "A";
}
-
- /* mark for close-on-exec */
- fcntl(epoll_fd, F_SETFD, FD_CLOEXEC);
+ if (fde->state & FDE_PENDING) {
+ state += "P";
+ }
+ if (fde->state & FDE_CREATED) {
+ state += "C";
+ }
+ if (fde->state & FDE_READ) {
+ state += "R";
+ }
+ if (fde->state & FDE_WRITE) {
+ state += "W";
+ }
+ if (fde->state & FDE_ERROR) {
+ state += "E";
+ }
+ if (fde->state & FDE_DONT_CLOSE) {
+ state += "D";
+ }
+ return android::base::StringPrintf("(fdevent %d %s)", fde->fd, state.c_str());
}
-static void fdevent_connect(fdevent *fde)
-{
- struct epoll_event ev;
-
- memset(&ev, 0, sizeof(ev));
- ev.events = 0;
- ev.data.ptr = fde;
-
-#if 0
- if(epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fde->fd, &ev)) {
- perror("epoll_ctl() failed\n");
- exit(1);
- }
-#endif
-}
-
-static void fdevent_disconnect(fdevent *fde)
-{
- struct epoll_event ev;
-
- memset(&ev, 0, sizeof(ev));
- ev.events = 0;
- ev.data.ptr = fde;
-
- /* technically we only need to delete if we
- ** were actively monitoring events, but let's
- ** be aggressive and do it anyway, just in case
- ** something's out of sync
- */
- epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fde->fd, &ev);
-}
-
-static void fdevent_update(fdevent *fde, unsigned events)
-{
- struct epoll_event ev;
- int active;
-
- active = (fde->state & FDE_EVENTMASK) != 0;
-
- memset(&ev, 0, sizeof(ev));
- ev.events = 0;
- ev.data.ptr = fde;
-
- if(events & FDE_READ) ev.events |= EPOLLIN;
- if(events & FDE_WRITE) ev.events |= EPOLLOUT;
- if(events & FDE_ERROR) ev.events |= (EPOLLERR | EPOLLHUP);
-
- fde->state = (fde->state & FDE_STATEMASK) | events;
-
- if(active) {
- /* we're already active. if we're changing to *no*
- ** events being monitored, we need to delete, otherwise
- ** we need to just modify
- */
- if(ev.events) {
- if(epoll_ctl(epoll_fd, EPOLL_CTL_MOD, fde->fd, &ev)) {
- perror("epoll_ctl() failed\n");
- exit(1);
- }
- } else {
- if(epoll_ctl(epoll_fd, EPOLL_CTL_DEL, fde->fd, &ev)) {
- perror("epoll_ctl() failed\n");
- exit(1);
- }
- }
- } else {
- /* we're not active. if we're watching events, we need
- ** to add, otherwise we can just do nothing
- */
- if(ev.events) {
- if(epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fde->fd, &ev)) {
- perror("epoll_ctl() failed\n");
- exit(1);
- }
- }
- }
-}
-
-static void fdevent_process()
-{
- struct epoll_event events[256];
- fdevent *fde;
- int i, n;
-
- n = epoll_wait(epoll_fd, events, 256, -1);
-
- if (n < 0) {
- if (errno == EINTR) return;
- perror("epoll_wait");
- exit(1);
- }
-
- for(i = 0; i < n; i++) {
- struct epoll_event *ev = events + i;
- fde = ev->data.ptr;
-
- if(ev->events & EPOLLIN) {
- fde->events |= FDE_READ;
- }
- if(ev->events & EPOLLOUT) {
- fde->events |= FDE_WRITE;
- }
- if(ev->events & (EPOLLERR | EPOLLHUP)) {
- fde->events |= FDE_ERROR;
- }
- if(fde->events) {
- if(fde->state & FDE_PENDING) continue;
- fde->state |= FDE_PENDING;
- fdevent_plist_enqueue(fde);
- }
- }
-}
-
-#else /* USE_SELECT */
-
-#if defined(_WIN32)
-#include <winsock2.h>
-#else
-#include <sys/select.h>
-#endif
-
-static fd_set read_fds;
-static fd_set write_fds;
-static fd_set error_fds;
-
-static int select_n = 0;
-
-static void fdevent_init(void)
-{
- FD_ZERO(&read_fds);
- FD_ZERO(&write_fds);
- FD_ZERO(&error_fds);
-}
-
-static void fdevent_connect(fdevent *fde)
-{
- if(fde->fd >= select_n) {
- select_n = fde->fd + 1;
- }
-}
-
-static void fdevent_disconnect(fdevent *fde)
-{
- int i, n;
-
- FD_CLR(fde->fd, &read_fds);
- FD_CLR(fde->fd, &write_fds);
- FD_CLR(fde->fd, &error_fds);
-
- for(n = 0, i = 0; i < select_n; i++) {
- if(fd_table[i] != 0) n = i;
- }
- select_n = n + 1;
-}
-
-static void fdevent_update(fdevent *fde, unsigned events)
-{
- if(events & FDE_READ) {
- FD_SET(fde->fd, &read_fds);
- } else {
- FD_CLR(fde->fd, &read_fds);
- }
- if(events & FDE_WRITE) {
- FD_SET(fde->fd, &write_fds);
- } else {
- FD_CLR(fde->fd, &write_fds);
- }
- if(events & FDE_ERROR) {
- FD_SET(fde->fd, &error_fds);
- } else {
- FD_CLR(fde->fd, &error_fds);
- }
-
- fde->state = (fde->state & FDE_STATEMASK) | events;
-}
-
-/* Looks at fd_table[] for bad FDs and sets bit in fds.
-** Returns the number of bad FDs.
-*/
-static int fdevent_fd_check(fd_set *fds)
-{
- int i, n = 0;
- fdevent *fde;
-
- for(i = 0; i < select_n; i++) {
- fde = fd_table[i];
- if(fde == 0) continue;
- if(fcntl(i, F_GETFL, NULL) < 0) {
- FD_SET(i, fds);
- n++;
- // fde->state |= FDE_DONT_CLOSE;
-
- }
- }
- return n;
-}
-
-#if !DEBUG
-static inline void dump_all_fds(const char* /* extra_msg */) {}
-#else
-static void dump_all_fds(const char *extra_msg)
-{
-int i;
- fdevent *fde;
- // per fd: 4 digits (but really: log10(FD_SETSIZE)), 1 staus, 1 blank
- char msg_buff[FD_SETSIZE*6 + 1], *pb=msg_buff;
- size_t max_chars = FD_SETSIZE * 6 + 1;
- int printed_out;
-#define SAFE_SPRINTF(...) \
- do { \
- printed_out = snprintf(pb, max_chars, __VA_ARGS__); \
- if (printed_out <= 0) { \
- D("... snprintf failed."); \
- return; \
- } \
- if (max_chars < (unsigned int)printed_out) { \
- D("... snprintf out of space."); \
- return; \
- } \
- pb += printed_out; \
- max_chars -= printed_out; \
- } while(0)
-
- for(i = 0; i < select_n; i++) {
- fde = fd_table[i];
- SAFE_SPRINTF("%d", i);
- if(fde == 0) {
- SAFE_SPRINTF("? ");
- continue;
- }
- if(fcntl(i, F_GETFL, NULL) < 0) {
- SAFE_SPRINTF("b");
- }
- SAFE_SPRINTF(" ");
- }
- D("%s fd_table[]->fd = {%s}", extra_msg, msg_buff);
-}
-#endif
-
-static void fdevent_process()
-{
- int i, n;
- fdevent *fde;
- unsigned events;
- fd_set rfd, wfd, efd;
-
- memcpy(&rfd, &read_fds, sizeof(fd_set));
- memcpy(&wfd, &write_fds, sizeof(fd_set));
- memcpy(&efd, &error_fds, sizeof(fd_set));
-
- dump_all_fds("pre select()");
-
- n = select(select_n, &rfd, &wfd, &efd, NULL);
- int saved_errno = errno;
- D("select() returned n=%d, errno=%d", n, n<0?saved_errno:0);
-
- dump_all_fds("post select()");
-
- if(n < 0) {
- switch(saved_errno) {
- case EINTR: return;
- case EBADF:
- // Can't trust the FD sets after an error.
- FD_ZERO(&wfd);
- FD_ZERO(&efd);
- FD_ZERO(&rfd);
- break;
- default:
- D("Unexpected select() error=%d", saved_errno);
- return;
- }
- }
- if(n <= 0) {
- // We fake a read, as the rest of the code assumes
- // that errors will be detected at that point.
- n = fdevent_fd_check(&rfd);
- }
-
- for(i = 0; (i < select_n) && (n > 0); i++) {
- events = 0;
- if(FD_ISSET(i, &rfd)) { events |= FDE_READ; n--; }
- if(FD_ISSET(i, &wfd)) { events |= FDE_WRITE; n--; }
- if(FD_ISSET(i, &efd)) { events |= FDE_ERROR; n--; }
-
- if(events) {
- fde = fd_table[i];
- if(fde == 0)
- FATAL("missing fde for fd %d\n", i);
-
- fde->events |= events;
-
- D("got events fde->fd=%d events=%04x, state=%04x",
- fde->fd, fde->events, fde->state);
- if(fde->state & FDE_PENDING) continue;
- fde->state |= FDE_PENDING;
- fdevent_plist_enqueue(fde);
- }
- }
-}
-
-#endif
-
-static void fdevent_register(fdevent *fde)
-{
- if(fde->fd < 0) {
- FATAL("bogus negative fd (%d)\n", fde->fd);
- }
-
- if(fde->fd >= fd_table_max) {
- int oldmax = fd_table_max;
- if(fde->fd > 32000) {
- FATAL("bogus huuuuge fd (%d)\n", fde->fd);
- }
- if(fd_table_max == 0) {
- fdevent_init();
- fd_table_max = 256;
- }
- while(fd_table_max <= fde->fd) {
- fd_table_max *= 2;
- }
- fd_table = reinterpret_cast<fdevent**>(
- realloc(fd_table, sizeof(fdevent*) * fd_table_max));
- if(fd_table == 0) {
- FATAL("could not expand fd_table to %d entries\n", fd_table_max);
- }
- memset(fd_table + oldmax, 0, sizeof(int) * (fd_table_max - oldmax));
- }
-
- fd_table[fde->fd] = fde;
-}
-
-static void fdevent_unregister(fdevent *fde)
-{
- if((fde->fd < 0) || (fde->fd >= fd_table_max)) {
- FATAL("fd out of range (%d)\n", fde->fd);
- }
-
- if(fd_table[fde->fd] != fde) {
- FATAL("fd_table out of sync [%d]\n", fde->fd);
- }
-
- fd_table[fde->fd] = 0;
-
- if(!(fde->state & FDE_DONT_CLOSE)) {
- dump_fde(fde, "close");
- adb_close(fde->fd);
- }
-}
-
-static void fdevent_plist_enqueue(fdevent *node)
-{
- fdevent *list = &list_pending;
-
- node->next = list;
- node->prev = list->prev;
- node->prev->next = node;
- list->prev = node;
-}
-
-static void fdevent_plist_remove(fdevent *node)
-{
- node->prev->next = node->next;
- node->next->prev = node->prev;
- node->next = 0;
- node->prev = 0;
-}
-
-static fdevent *fdevent_plist_dequeue(void)
-{
- fdevent *list = &list_pending;
- fdevent *node = list->next;
-
- if(node == list) return 0;
-
- list->next = node->next;
- list->next->prev = list;
- node->next = 0;
- node->prev = 0;
-
- return node;
-}
-
-static void fdevent_call_fdfunc(fdevent* fde)
-{
- unsigned events = fde->events;
- fde->events = 0;
- if(!(fde->state & FDE_PENDING)) return;
- fde->state &= (~FDE_PENDING);
- dump_fde(fde, "callback");
- fde->func(fde->fd, events, fde->arg);
-}
-
-#if !ADB_HOST
-static void fdevent_subproc_event_func(int fd, unsigned ev,
- void* /* userdata */)
-{
-
- D("subproc handling on fd=%d ev=%04x", fd, ev);
-
- // Hook oneself back into the fde's suitable for select() on read.
- if((fd < 0) || (fd >= fd_table_max)) {
- FATAL("fd %d out of range for fd_table \n", fd);
- }
- fdevent *fde = fd_table[fd];
- fdevent_add(fde, FDE_READ);
-
- if(ev & FDE_READ){
- int subproc_fd;
-
- if(!ReadFdExactly(fd, &subproc_fd, sizeof(subproc_fd))) {
- FATAL("Failed to read the subproc's fd from fd=%d\n", fd);
- }
- if((subproc_fd < 0) || (subproc_fd >= fd_table_max)) {
- D("subproc_fd %d out of range 0, fd_table_max=%d",
- subproc_fd, fd_table_max);
- return;
- }
- fdevent *subproc_fde = fd_table[subproc_fd];
- if(!subproc_fde) {
- D("subproc_fd %d cleared from fd_table", subproc_fd);
- return;
- }
- if(subproc_fde->fd != subproc_fd) {
- // Already reallocated?
- D("subproc_fd %d != fd_table[].fd %d", subproc_fd, subproc_fde->fd);
- return;
- }
-
- subproc_fde->force_eof = 1;
-
- int rcount = 0;
- ioctl(subproc_fd, FIONREAD, &rcount);
- D("subproc with fd=%d has rcount=%d err=%d",
- subproc_fd, rcount, errno);
-
- if(rcount) {
- // If there is data left, it will show up in the select().
- // This works because there is no other thread reading that
- // data when in this fd_func().
- return;
- }
-
- D("subproc_fde.state=%04x", subproc_fde->state);
- subproc_fde->events |= FDE_READ;
- if(subproc_fde->state & FDE_PENDING) {
- return;
- }
- subproc_fde->state |= FDE_PENDING;
- fdevent_call_fdfunc(subproc_fde);
- }
-}
-
-void fdevent_subproc_setup()
-{
- int s[2];
-
- if(adb_socketpair(s)) {
- FATAL("cannot create shell-exit socket-pair\n");
- }
- D("socketpair: (%d,%d)", s[0], s[1]);
-
- SHELL_EXIT_NOTIFY_FD = s[0];
- fdevent *fde;
- fde = fdevent_create(s[1], fdevent_subproc_event_func, NULL);
- if(!fde)
- FATAL("cannot create fdevent for shell-exit handler\n");
- fdevent_add(fde, FDE_READ);
-}
-#endif // !ADB_HOST
-
fdevent *fdevent_create(int fd, fd_func func, void *arg)
{
fdevent *fde = (fdevent*) malloc(sizeof(fdevent));
@@ -602,98 +105,234 @@
{
if(fde == 0) return;
if(!(fde->state & FDE_CREATED)) {
- FATAL("fde %p not created by fdevent_create()\n", fde);
+ LOG(FATAL) << "destroying fde not created by fdevent_create(): " << dump_fde(fde);
}
fdevent_remove(fde);
free(fde);
}
-void fdevent_install(fdevent *fde, int fd, fd_func func, void *arg)
-{
+void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
+ CHECK_GE(fd, 0);
memset(fde, 0, sizeof(fdevent));
fde->state = FDE_ACTIVE;
fde->fd = fd;
- fde->force_eof = 0;
fde->func = func;
fde->arg = arg;
-
-#if !defined(_WIN32)
- fcntl(fd, F_SETFL, O_NONBLOCK);
-#endif
- fdevent_register(fde);
- dump_fde(fde, "connect");
- fdevent_connect(fde);
- fde->state |= FDE_ACTIVE;
+ if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) {
+ // Here is not proper to handle the error. If it fails here, some error is
+ // likely to be detected by poll(), then we can let the callback function
+ // to handle it.
+ LOG(ERROR) << "failed to fcntl(" << fd << ") to be nonblock";
+ }
+ auto pair = g_poll_node_map.emplace(fde->fd, PollNode(fde));
+ CHECK(pair.second) << "install existing fd " << fd;
+ D("fdevent_install %s", dump_fde(fde).c_str());
}
-void fdevent_remove(fdevent *fde)
-{
- if(fde->state & FDE_PENDING) {
- fdevent_plist_remove(fde);
+void fdevent_remove(fdevent* fde) {
+ D("fdevent_remove %s", dump_fde(fde).c_str());
+ if (fde->state & FDE_ACTIVE) {
+ g_poll_node_map.erase(fde->fd);
+ if (fde->state & FDE_PENDING) {
+ g_pending_list.remove(fde);
+ }
+ if (!(fde->state & FDE_DONT_CLOSE)) {
+ adb_close(fde->fd);
+ fde->fd = -1;
+ }
+ fde->state = 0;
+ fde->events = 0;
}
-
- if(fde->state & FDE_ACTIVE) {
- fdevent_disconnect(fde);
- dump_fde(fde, "disconnect");
- fdevent_unregister(fde);
- }
-
- fde->state = 0;
- fde->events = 0;
}
-
-void fdevent_set(fdevent *fde, unsigned events)
-{
- events &= FDE_EVENTMASK;
-
- if((fde->state & FDE_EVENTMASK) == events) return;
-
- if(fde->state & FDE_ACTIVE) {
- fdevent_update(fde, events);
- dump_fde(fde, "update");
+static void fdevent_update(fdevent* fde, unsigned events) {
+ auto it = g_poll_node_map.find(fde->fd);
+ CHECK(it != g_poll_node_map.end());
+ PollNode& node = it->second;
+ if (events & FDE_READ) {
+ node.pollfd.events |= POLLIN;
+ } else {
+ node.pollfd.events &= ~POLLIN;
}
+ if (events & FDE_WRITE) {
+ node.pollfd.events |= POLLOUT;
+ } else {
+ node.pollfd.events &= ~POLLOUT;
+ }
fde->state = (fde->state & FDE_STATEMASK) | events;
+}
- if(fde->state & FDE_PENDING) {
- /* if we're pending, make sure
- ** we don't signal an event that
- ** is no longer wanted.
- */
- fde->events &= (~events);
- if(fde->events == 0) {
- fdevent_plist_remove(fde);
- fde->state &= (~FDE_PENDING);
+void fdevent_set(fdevent* fde, unsigned events) {
+ events &= FDE_EVENTMASK;
+ if ((fde->state & FDE_EVENTMASK) == events) {
+ return;
+ }
+ if (fde->state & FDE_ACTIVE) {
+ fdevent_update(fde, events);
+ D("fdevent_set: %s, events = %u", dump_fde(fde).c_str(), events);
+
+ if (fde->state & FDE_PENDING) {
+ // If we are pending, make sure we don't signal an event that is no longer wanted.
+ fde->events &= ~events;
+ if (fde->events == 0) {
+ g_pending_list.remove(fde);
+ fde->state &= ~FDE_PENDING;
+ }
}
}
}
-void fdevent_add(fdevent *fde, unsigned events)
-{
- fdevent_set(
- fde, (fde->state & FDE_EVENTMASK) | (events & FDE_EVENTMASK));
+void fdevent_add(fdevent* fde, unsigned events) {
+ fdevent_set(fde, (fde->state & FDE_EVENTMASK) | events);
}
-void fdevent_del(fdevent *fde, unsigned events)
-{
- fdevent_set(
- fde, (fde->state & FDE_EVENTMASK) & (~(events & FDE_EVENTMASK)));
+void fdevent_del(fdevent* fde, unsigned events) {
+ fdevent_set(fde, (fde->state & FDE_EVENTMASK) & ~events);
}
+static std::string dump_pollfds(const std::vector<pollfd>& pollfds) {
+ std::string result;
+ for (auto& pollfd : pollfds) {
+ std::string op;
+ if (pollfd.events & POLLIN) {
+ op += "R";
+ }
+ if (pollfd.events & POLLOUT) {
+ op += "W";
+ }
+ android::base::StringAppendF(&result, " %d(%s)", pollfd.fd, op.c_str());
+ }
+ return result;
+}
+
+static void fdevent_process() {
+ std::vector<pollfd> pollfds;
+ for (auto it = g_poll_node_map.begin(); it != g_poll_node_map.end(); ++it) {
+ pollfds.push_back(it->second.pollfd);
+ }
+ CHECK_GT(pollfds.size(), 0u);
+ D("poll(), pollfds = %s", dump_pollfds(pollfds).c_str());
+ int ret = TEMP_FAILURE_RETRY(poll(&pollfds[0], pollfds.size(), -1));
+ if (ret == -1) {
+ PLOG(ERROR) << "poll(), ret = " << ret;
+ return;
+ }
+ for (auto& pollfd : pollfds) {
+ unsigned events = 0;
+ if (pollfd.revents & POLLIN) {
+ events |= FDE_READ;
+ }
+ if (pollfd.revents & POLLOUT) {
+ events |= FDE_WRITE;
+ }
+ if (pollfd.revents & (POLLERR | POLLHUP | POLLNVAL)) {
+ // We fake a read, as the rest of the code assumes that errors will
+ // be detected at that point.
+ events |= FDE_READ | FDE_ERROR;
+ }
+ if (events != 0) {
+ auto it = g_poll_node_map.find(pollfd.fd);
+ CHECK(it != g_poll_node_map.end());
+ fdevent* fde = it->second.fde;
+ CHECK_EQ(fde->fd, pollfd.fd);
+ fde->events |= events;
+ D("%s got events %x", dump_fde(fde).c_str(), events);
+ fde->state |= FDE_PENDING;
+ g_pending_list.push_back(fde);
+ }
+ }
+}
+
+static void fdevent_call_fdfunc(fdevent* fde)
+{
+ unsigned events = fde->events;
+ fde->events = 0;
+ if(!(fde->state & FDE_PENDING)) return;
+ fde->state &= (~FDE_PENDING);
+ D("fdevent_call_fdfunc %s", dump_fde(fde).c_str());
+ fde->func(fde->fd, events, fde->arg);
+}
+
+#if !ADB_HOST
+static void fdevent_subproc_event_func(int fd, unsigned ev,
+ void* /* userdata */)
+{
+
+ D("subproc handling on fd = %d, ev = %x", fd, ev);
+
+ CHECK_GE(fd, 0);
+
+ if (ev & FDE_READ) {
+ int subproc_fd;
+
+ if(!ReadFdExactly(fd, &subproc_fd, sizeof(subproc_fd))) {
+ LOG(FATAL) << "Failed to read the subproc's fd from " << fd;
+ }
+ auto it = g_poll_node_map.find(subproc_fd);
+ if (it == g_poll_node_map.end()) {
+ D("subproc_fd %d cleared from fd_table", subproc_fd);
+ return;
+ }
+ fdevent* subproc_fde = it->second.fde;
+ if(subproc_fde->fd != subproc_fd) {
+ // Already reallocated?
+ D("subproc_fd(%d) != subproc_fde->fd(%d)", subproc_fd, subproc_fde->fd);
+ return;
+ }
+
+ subproc_fde->force_eof = 1;
+
+ int rcount = 0;
+ ioctl(subproc_fd, FIONREAD, &rcount);
+ D("subproc with fd %d has rcount=%d, err=%d", subproc_fd, rcount, errno);
+ if (rcount != 0) {
+ // If there is data left, it will show up in the select().
+ // This works because there is no other thread reading that
+ // data when in this fd_func().
+ return;
+ }
+
+ D("subproc_fde %s", dump_fde(subproc_fde).c_str());
+ subproc_fde->events |= FDE_READ;
+ if(subproc_fde->state & FDE_PENDING) {
+ return;
+ }
+ subproc_fde->state |= FDE_PENDING;
+ fdevent_call_fdfunc(subproc_fde);
+ }
+}
+
+void fdevent_subproc_setup()
+{
+ int s[2];
+
+ if(adb_socketpair(s)) {
+ PLOG(FATAL) << "cannot create shell-exit socket-pair";
+ }
+ D("fdevent_subproc: socket pair (%d, %d)", s[0], s[1]);
+
+ SHELL_EXIT_NOTIFY_FD = s[0];
+ fdevent *fde = fdevent_create(s[1], fdevent_subproc_event_func, NULL);
+ CHECK(fde != nullptr) << "cannot create fdevent for shell-exit handler";
+ fdevent_add(fde, FDE_READ);
+}
+#endif // !ADB_HOST
+
void fdevent_loop()
{
- fdevent *fde;
#if !ADB_HOST
fdevent_subproc_setup();
#endif // !ADB_HOST
while (true) {
- D("--- ---- waiting for events");
+ D("--- --- waiting for events");
fdevent_process();
- while((fde = fdevent_plist_dequeue())) {
+ while (!g_pending_list.empty()) {
+ fdevent* fde = g_pending_list.front();
+ g_pending_list.pop_front();
fdevent_call_fdfunc(fde);
}
}
diff --git a/adb/fdevent_test.cpp b/adb/fdevent_test.cpp
index f7622b5..33034d8 100644
--- a/adb/fdevent_test.cpp
+++ b/adb/fdevent_test.cpp
@@ -18,6 +18,7 @@
#include <gtest/gtest.h>
+#include <limits>
#include <queue>
#include <string>
#include <vector>
@@ -50,9 +51,13 @@
public:
FdHandler(int read_fd, int write_fd) : read_fd_(read_fd), write_fd_(write_fd) {
fdevent_install(&read_fde_, read_fd_, FdEventCallback, this);
- fdevent_add(&read_fde_, FDE_READ | FDE_ERROR);
+ fdevent_add(&read_fde_, FDE_READ);
fdevent_install(&write_fde_, write_fd_, FdEventCallback, this);
- fdevent_add(&write_fde_, FDE_ERROR);
+ }
+
+ ~FdHandler() {
+ fdevent_remove(&read_fde_);
+ fdevent_remove(&write_fde_);
}
private:
@@ -150,3 +155,44 @@
ASSERT_EQ(0, pthread_kill(thread, SIGUSR1));
ASSERT_EQ(0, pthread_join(thread, nullptr));
}
+
+struct InvalidFdArg {
+ fdevent fde;
+ unsigned expected_events;
+ size_t* happened_event_count;
+};
+
+static void InvalidFdEventCallback(int fd, unsigned events, void* userdata) {
+ InvalidFdArg* arg = reinterpret_cast<InvalidFdArg*>(userdata);
+ ASSERT_EQ(arg->expected_events, events);
+ fdevent_remove(&arg->fde);
+ if (++*(arg->happened_event_count) == 2) {
+ pthread_exit(nullptr);
+ }
+}
+
+void InvalidFdThreadFunc(void*) {
+ const int INVALID_READ_FD = std::numeric_limits<int>::max() - 1;
+ size_t happened_event_count = 0;
+ InvalidFdArg read_arg;
+ read_arg.expected_events = FDE_READ | FDE_ERROR;
+ read_arg.happened_event_count = &happened_event_count;
+ fdevent_install(&read_arg.fde, INVALID_READ_FD, InvalidFdEventCallback, &read_arg);
+ fdevent_add(&read_arg.fde, FDE_READ);
+
+ const int INVALID_WRITE_FD = std::numeric_limits<int>::max();
+ InvalidFdArg write_arg;
+ write_arg.expected_events = FDE_READ | FDE_ERROR;
+ write_arg.happened_event_count = &happened_event_count;
+ fdevent_install(&write_arg.fde, INVALID_WRITE_FD, InvalidFdEventCallback, &write_arg);
+ fdevent_add(&write_arg.fde, FDE_WRITE);
+ fdevent_loop();
+}
+
+TEST(fdevent, invalid_fd) {
+ pthread_t thread;
+ ASSERT_EQ(0, pthread_create(&thread, nullptr,
+ reinterpret_cast<void* (*)(void*)>(InvalidFdThreadFunc),
+ nullptr));
+ ASSERT_EQ(0, pthread_join(thread, nullptr));
+}
diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp
index 0274ae3..f1bc36d 100644
--- a/adb/shell_service.cpp
+++ b/adb/shell_service.cpp
@@ -413,6 +413,14 @@
D("closing FD %d", dead_sfd->fd());
FD_CLR(dead_sfd->fd(), &master_read_set);
FD_CLR(dead_sfd->fd(), &master_write_set);
+ if (dead_sfd == &protocol_sfd_) {
+ // Using SIGHUP is a decent general way to indicate that the
+ // controlling process is going away. If specific signals are
+ // needed (e.g. SIGINT), pass those through the shell protocol
+ // and only fall back on this for unexpected closures.
+ D("protocol FD died, sending SIGHUP to pid %d", pid_);
+ kill(pid_, SIGHUP);
+ }
dead_sfd->Reset();
}
}
diff --git a/adb/test_device.py b/adb/test_device.py
index fedd2d7..4452eed 100644
--- a/adb/test_device.py
+++ b/adb/test_device.py
@@ -23,6 +23,7 @@
import random
import shlex
import shutil
+import signal
import subprocess
import tempfile
import unittest
@@ -196,6 +197,34 @@
self.assertEqual('foo' + self.device.linesep, result[1])
self.assertEqual('bar' + self.device.linesep, result[2])
+ def test_non_interactive_sigint(self):
+ """Tests that SIGINT in a non-interactive shell kills the process.
+
+ This requires the shell protocol in order to detect the broken
+ pipe; raw data transfer mode will only see the break once the
+ subprocess tries to read or write.
+
+ Bug: http://b/23825725
+ """
+ if self.device.SHELL_PROTOCOL_FEATURE not in self.device.features:
+ raise unittest.SkipTest('shell protocol unsupported on this device')
+
+ # Start a long-running process.
+ sleep_proc = subprocess.Popen(
+ self.device.adb_cmd + shlex.split('shell echo $$; sleep 60'),
+ stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT)
+ remote_pid = sleep_proc.stdout.readline().strip()
+ self.assertIsNone(sleep_proc.returncode, 'subprocess terminated early')
+ proc_query = shlex.split('ps {0} | grep {0}'.format(remote_pid))
+
+ # Verify that the process is running, send signal, verify it stopped.
+ self.device.shell(proc_query)
+ os.kill(sleep_proc.pid, signal.SIGINT)
+ sleep_proc.communicate()
+ self.assertEqual(1, self.device.shell_nocheck(proc_query)[0],
+ 'subprocess failed to terminate')
+
class ArgumentEscapingTest(DeviceTest):
def test_shell_escaping(self):
diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender
index 7f9062a..d0d6772 100755
--- a/crash_reporter/crash_sender
+++ b/crash_reporter/crash_sender
@@ -63,11 +63,8 @@
# Must be stateful to enable testing kernel crashes.
PAUSE_CRASH_SENDING="${CRASH_STATE_DIR}/lock/crash_sender_paused"
-# URL to send official build crash reports to.
-REPORT_UPLOAD_PROD_URL="https://clients2.google.com/cr/report"
-
# Path to a directory of restricted certificates which includes
-# a certificate for ${REPORT_UPLOAD_PROD_URL}.
+# a certificate for the crash server.
RESTRICTED_CERTIFICATES_PATH="/system/etc/security/cacerts"
# File whose existence implies we're running and not to start again.
@@ -79,6 +76,9 @@
# Set this to 1 to allow uploading of device coredumps.
DEVCOREDUMP_UPLOAD_FLAG_FILE="${CRASH_STATE_DIR}/device_coredump_upload_allowed"
+# The weave configuration file.
+WEAVE_CONF_FILE="/etc/weaved/weaved.conf"
+
# The syslog tag for all logging we emit.
TAG="$(basename $0)[$$]"
@@ -180,10 +180,21 @@
}
# Generate a uniform random number in 0..max-1.
+# POSIX arithmetic expansion requires support of at least signed long integers.
+# On 32-bit systems, that may mean 32-bit signed integers, in which case the
+# 32-bit random number read from /dev/urandom may be interpreted as negative
+# when used inside an arithmetic expansion (since the high bit might be set).
+# mksh at least is known to behave this way.
+# For this case, simply take the absolute value, which will still give a
+# roughly uniform random distribution for the modulo (as we are merely ignoring
+# the high/sign bit).
+# See corresponding Arithmetic Expansion and Arithmetic Expression sections:
+# POSIX: http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_04
+# mksh: http://linux.die.net/man/1/mksh
generate_uniform_random() {
local max=$1
local random="$(od -An -N4 -tu /dev/urandom)"
- echo $((random % max))
+ echo $(((random < 0 ? -random : random) % max))
}
# Check if sending a crash now does not exceed the maximum 24hr rate and
@@ -277,7 +288,7 @@
local report_payload="$(get_key_value "${meta_path}" "payload")"
local kind="$(get_kind "${meta_path}")"
local exec_name="$(get_key_value "${meta_path}" "exec_name")"
- local url="${REPORT_UPLOAD_PROD_URL}"
+ local url="$(getprop crash_reporter.server)"
local brillo_version="$(get_key_value "${meta_path}" "ver")"
local hwclass="$(get_hardware_class)"
local write_payload_size="$(get_key_value "${meta_path}" "payload_size")"
@@ -288,6 +299,13 @@
local version="$(get_key_value "${meta_path}" "upload_var_ver")"
local upload_prefix="$(get_key_value "${meta_path}" "upload_prefix")"
local guid
+ local model_manifest_id="$(get_key_value "${WEAVE_CONF_FILE}" "model_id")"
+
+ # If crash_reporter.server is not set return with an error.
+ if [ -z "${url}" ]; then
+ lecho "Configuration error: crash_reporter.server not set."
+ return 1
+ fi
set -- \
-F "write_payload_size=${write_payload_size}" \
@@ -441,6 +459,7 @@
-F "ver=${version}" \
-F "hwclass=${hwclass}" \
-F "exec_name=${exec_name}" \
+ -F "model_manifest_id=${model_manifest_id}" \
${image_type:+-F "image_type=${image_type}"} \
${boot_mode:+-F "boot_mode=${boot_mode}"} \
${error_type:+-F "error_type=${error_type}"} \
diff --git a/init/perfboot.py b/init/perfboot.py
index 2a17ab6..91e6c2b 100755
--- a/init/perfboot.py
+++ b/init/perfboot.py
@@ -92,7 +92,7 @@
self._interval = interval
self._device = device
self._temp_paths = device.shell(
- ['ls', '/sys/class/thermal/thermal_zone*/temp']).splitlines()
+ ['ls', '/sys/class/thermal/thermal_zone*/temp'])[0].splitlines()
self._product = device.get_prop('ro.build.product')
self._waited = False
@@ -109,7 +109,7 @@
def _get_cpu_temp(self, threshold):
max_temp = 0
for temp_path in self._temp_paths:
- temp = int(self._device.shell(['cat', temp_path]).rstrip())
+ temp = int(self._device.shell(['cat', temp_path])[0].rstrip())
max_temp = max(max_temp, temp)
if temp >= threshold:
return temp
@@ -173,7 +173,7 @@
device.wait()
device.shell(['rm', '-rf', '/system/data/dropbox'])
original_dropbox_max_files = device.shell(
- ['settings', 'get', 'global', 'dropbox_max_files']).rstrip()
+ ['settings', 'get', 'global', 'dropbox_max_files'])[0].rstrip()
device.shell(['settings', 'put', 'global', 'dropbox_max_files', '0'])
return original_dropbox_max_files
@@ -244,7 +244,8 @@
"""Drop unknown tags not listed in device's event-log-tags file."""
device.wait()
supported_tags = set()
- for l in device.shell(['cat', '/system/etc/event-log-tags']).splitlines():
+ for l in device.shell(
+ ['cat', '/system/etc/event-log-tags'])[0].splitlines():
tokens = l.split(' ')
if len(tokens) >= 2:
supported_tags.add(tokens[1])
diff --git a/metricsd/constants.h b/metricsd/constants.h
index 15c15d9..717e5d2 100644
--- a/metricsd/constants.h
+++ b/metricsd/constants.h
@@ -19,10 +19,12 @@
namespace metrics {
static const char kMetricsDirectory[] = "/data/misc/metrics/";
-static const char kMetricsEventsFilePath[] = "/data/misc/metrics/uma-events";
-static const char kMetricsGUIDFilePath[] = "/data/misc/metrics/Sysinfo.GUID";
+static const char kMetricsEventsFileName[] = "uma-events";
+static const char kMetricsGUIDFileName[] = "Sysinfo.GUID";
static const char kMetricsServer[] = "https://clients4.google.com/uma/v2";
-static const char kConsentFilePath[] = "/data/misc/metrics/enabled";
+static const char kConsentFileName[] = "enabled";
+static const char kStagedLogName[] = "staged_log";
+static const char kFailedUploadCountName[] = "failed_upload_count";
static const char kDefaultVersion[] = "0.0.0.0";
// System properties used.
diff --git a/metricsd/include/metrics/metrics_library.h b/metricsd/include/metrics/metrics_library.h
index a956b69..26df2f4 100644
--- a/metricsd/include/metrics/metrics_library.h
+++ b/metricsd/include/metrics/metrics_library.h
@@ -22,6 +22,7 @@
#include <unistd.h>
#include <base/compiler_specific.h>
+#include <base/files/file_path.h>
#include <base/macros.h>
#include <base/memory/scoped_ptr.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
@@ -122,6 +123,7 @@
private:
friend class CMetricsLibraryTest;
friend class MetricsLibraryTest;
+ friend class UploadServiceTest;
FRIEND_TEST(MetricsLibraryTest, AreMetricsEnabled);
FRIEND_TEST(MetricsLibraryTest, FormatChromeMessage);
FRIEND_TEST(MetricsLibraryTest, FormatChromeMessageTooLong);
@@ -129,8 +131,7 @@
FRIEND_TEST(MetricsLibraryTest, SendMessageToChrome);
FRIEND_TEST(MetricsLibraryTest, SendMessageToChromeUMAEventsBadFileLocation);
- void InitForTest(const std::string& uma_events_file,
- const std::string& consent_file);
+ void InitForTest(const base::FilePath& metrics_directory);
// Sets |*result| to whether or not the |mounts_file| indicates that
// the |device_name| is currently mounted. Uses |buffer| of
@@ -146,8 +147,8 @@
// Cached state of whether or not metrics were enabled.
static bool cached_enabled_;
- std::string uma_events_file_;
- std::string consent_file_;
+ base::FilePath uma_events_file_;
+ base::FilePath consent_file_;
DISALLOW_COPY_AND_ASSIGN(MetricsLibrary);
};
diff --git a/metricsd/metrics_client.cc b/metricsd/metrics_client.cc
index f658b22..78174ef 100644
--- a/metricsd/metrics_client.cc
+++ b/metricsd/metrics_client.cc
@@ -140,11 +140,13 @@
}
static int DumpLogs() {
- printf("Metrics from %s\n\n", metrics::kMetricsEventsFilePath);
+ base::FilePath events_file = base::FilePath(
+ metrics::kMetricsDirectory).Append(metrics::kMetricsEventsFileName);
+ printf("Metrics from %s\n\n", events_file.value().data());
ScopedVector<metrics::MetricSample> metrics;
- metrics::SerializationUtils::ReadMetricsFromFile(
- metrics::kMetricsEventsFilePath, &metrics);
+ metrics::SerializationUtils::ReadMetricsFromFile(events_file.value(),
+ &metrics);
for (ScopedVector<metrics::MetricSample>::const_iterator i = metrics.begin();
i != metrics.end(); ++i) {
diff --git a/metricsd/metrics_daemon.cc b/metricsd/metrics_daemon.cc
index e35bc28..de7f2ea 100644
--- a/metricsd/metrics_daemon.cc
+++ b/metricsd/metrics_daemon.cc
@@ -187,10 +187,10 @@
void MetricsDaemon::RunUploaderTest() {
upload_service_.reset(new UploadService(
- new SystemProfileCache(true, base::FilePath(config_root_)),
+ new SystemProfileCache(true, metrics_directory_),
metrics_lib_,
server_));
- upload_service_->Init(upload_interval_, metrics_file_);
+ upload_service_->Init(upload_interval_, metrics_directory_);
upload_service_->UploadEvent();
}
@@ -223,18 +223,16 @@
const string& cpuinfo_max_freq_path,
const base::TimeDelta& upload_interval,
const string& server,
- const string& metrics_file,
- const string& config_root) {
+ const base::FilePath& metrics_directory) {
CHECK(metrics_lib);
testing_ = testing;
uploader_active_ = uploader_active;
dbus_enabled_ = dbus_enabled;
- config_root_ = config_root;
+ metrics_directory_ = metrics_directory;
metrics_lib_ = metrics_lib;
upload_interval_ = upload_interval;
server_ = server;
- metrics_file_ = metrics_file;
// Get ticks per second (HZ) on this system.
// Sysconf cannot fail, so no sanity checks are needed.
@@ -337,7 +335,7 @@
if (uploader_active_) {
upload_service_.reset(
new UploadService(new SystemProfileCache(), metrics_lib_, server_));
- upload_service_->Init(upload_interval_, metrics_file_);
+ upload_service_->Init(upload_interval_, metrics_directory_);
}
return EX_OK;
diff --git a/metricsd/metrics_daemon.h b/metricsd/metrics_daemon.h
index 7f7ea63..b363c5e 100644
--- a/metricsd/metrics_daemon.h
+++ b/metricsd/metrics_daemon.h
@@ -50,8 +50,7 @@
const std::string& scaling_max_freq_path,
const base::TimeDelta& upload_interval,
const std::string& server,
- const std::string& metrics_file,
- const std::string& config_root);
+ const base::FilePath& metrics_directory);
// Initializes DBus and MessageLoop variables before running the MessageLoop.
int OnInit() override;
@@ -268,7 +267,7 @@
bool dbus_enabled_;
// Root of the configuration files to use.
- std::string config_root_;
+ base::FilePath metrics_directory_;
// The metrics library handle.
MetricsLibraryInterface* metrics_lib_;
@@ -331,7 +330,6 @@
base::TimeDelta upload_interval_;
std::string server_;
- std::string metrics_file_;
scoped_ptr<UploadService> upload_service_;
};
diff --git a/metricsd/metrics_daemon_main.cc b/metricsd/metrics_daemon_main.cc
index 43046f8..c2e794e 100644
--- a/metricsd/metrics_daemon_main.cc
+++ b/metricsd/metrics_daemon_main.cc
@@ -75,11 +75,9 @@
DEFINE_string(server,
metrics::kMetricsServer,
"Server to upload the metrics to. (needs -uploader)");
- DEFINE_string(metrics_file,
- metrics::kMetricsEventsFilePath,
- "File to use as a proxy for uploading the metrics");
- DEFINE_string(config_root,
- "/", "Root of the configuration files (testing only)");
+ DEFINE_string(metrics_directory,
+ metrics::kMetricsDirectory,
+ "Root of the configuration files (testing only)");
chromeos::FlagHelper::Init(argc, argv, "Chromium OS Metrics Daemon");
@@ -103,8 +101,7 @@
kCpuinfoMaxFreqPath,
base::TimeDelta::FromSeconds(FLAGS_upload_interval_secs),
FLAGS_server,
- FLAGS_metrics_file,
- FLAGS_config_root);
+ base::FilePath(FLAGS_metrics_directory));
if (FLAGS_uploader_test) {
daemon.RunUploaderTest();
diff --git a/metricsd/metrics_daemon_test.cc b/metricsd/metrics_daemon_test.cc
index 4ef097e..476d0f3 100644
--- a/metricsd/metrics_daemon_test.cc
+++ b/metricsd/metrics_daemon_test.cc
@@ -87,8 +87,7 @@
cpu_max_freq_path_.value(),
base::TimeDelta::FromMinutes(30),
metrics::kMetricsServer,
- metrics::kMetricsEventsFilePath,
- "/");
+ temp_dir_.path());
}
// Adds a metrics library mock expectation that the specified metric
diff --git a/metricsd/metrics_library.cc b/metricsd/metrics_library.cc
index 5687f1b..6449a24 100644
--- a/metricsd/metrics_library.cc
+++ b/metricsd/metrics_library.cc
@@ -56,7 +56,7 @@
time_t MetricsLibrary::cached_enabled_time_ = 0;
bool MetricsLibrary::cached_enabled_ = false;
-MetricsLibrary::MetricsLibrary() : consent_file_(metrics::kConsentFilePath) {}
+MetricsLibrary::MetricsLibrary() {}
MetricsLibrary::~MetricsLibrary() {}
// We take buffer and buffer_size as parameters in order to simplify testing
@@ -131,19 +131,20 @@
time_t this_check_time = time(nullptr);
if (this_check_time != cached_enabled_time_) {
cached_enabled_time_ = this_check_time;
- cached_enabled_ = stat(consent_file_.c_str(), &stat_buffer) >= 0;
+ cached_enabled_ = stat(consent_file_.value().data(), &stat_buffer) >= 0;
}
return cached_enabled_;
}
void MetricsLibrary::Init() {
- uma_events_file_ = metrics::kMetricsEventsFilePath;
+ base::FilePath dir = base::FilePath(metrics::kMetricsDirectory);
+ uma_events_file_ = dir.Append(metrics::kMetricsEventsFileName);
+ consent_file_ = dir.Append(metrics::kConsentFileName);
}
-void MetricsLibrary::InitForTest(const std::string& uma_events_file,
- const std::string& consent_file) {
- uma_events_file_ = uma_events_file;
- consent_file_ = consent_file;
+void MetricsLibrary::InitForTest(const base::FilePath& metrics_directory) {
+ uma_events_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
+ consent_file_ = metrics_directory.Append(metrics::kConsentFileName);
}
bool MetricsLibrary::SendToUMA(const std::string& name,
@@ -154,30 +155,32 @@
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::HistogramSample(name, sample, min, max, nbuckets)
.get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendEnumToUMA(const std::string& name, int sample,
int max) {
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::LinearHistogramSample(name, sample, max).get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendSparseToUMA(const std::string& name, int sample) {
return metrics::SerializationUtils::WriteMetricToFile(
*metrics::MetricSample::SparseHistogramSample(name, sample).get(),
- metrics::kMetricsEventsFilePath);
+ uma_events_file_.value());
}
bool MetricsLibrary::SendUserActionToUMA(const std::string& action) {
return metrics::SerializationUtils::WriteMetricToFile(
- *metrics::MetricSample::UserActionSample(action).get(), metrics::kMetricsEventsFilePath);
+ *metrics::MetricSample::UserActionSample(action).get(),
+ uma_events_file_.value());
}
bool MetricsLibrary::SendCrashToUMA(const char *crash_kind) {
return metrics::SerializationUtils::WriteMetricToFile(
- *metrics::MetricSample::CrashSample(crash_kind).get(), metrics::kMetricsEventsFilePath);
+ *metrics::MetricSample::CrashSample(crash_kind).get(),
+ uma_events_file_.value());
}
bool MetricsLibrary::SendCrosEventToUMA(const std::string& event) {
diff --git a/metricsd/metrics_library_test.cc b/metricsd/metrics_library_test.cc
index 7ade6ee..f300d17 100644
--- a/metricsd/metrics_library_test.cc
+++ b/metricsd/metrics_library_test.cc
@@ -28,19 +28,17 @@
protected:
virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- consent_file_ = temp_dir_.path().Append("consent");
- uma_events_file_ = temp_dir_.path().Append("events");
- lib_.InitForTest(uma_events_file_.value(), consent_file_.value());
- EXPECT_EQ(0, WriteFile(uma_events_file_, "", 0));
+ lib_.InitForTest(temp_dir_.path());
+ EXPECT_EQ(0, WriteFile(lib_.uma_events_file_, "", 0));
// Defeat metrics enabled caching between tests.
lib_.cached_enabled_time_ = 0;
}
void SetMetricsConsent(bool enabled) {
if (enabled) {
- ASSERT_EQ(base::WriteFile(consent_file_, "", 0), 0);
+ ASSERT_EQ(base::WriteFile(lib_.consent_file_, "", 0), 0);
} else {
- ASSERT_TRUE(base::DeleteFile(consent_file_, false));
+ ASSERT_TRUE(base::DeleteFile(lib_.consent_file_, false));
}
}
@@ -49,8 +47,6 @@
MetricsLibrary lib_;
base::ScopedTempDir temp_dir_;
- base::FilePath consent_file_;
- base::FilePath uma_events_file_;
};
TEST_F(MetricsLibraryTest, AreMetricsEnabledFalse) {
diff --git a/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc
index 8635fb0..e3f6339 100644
--- a/metricsd/uploader/system_profile_cache.cc
+++ b/metricsd/uploader/system_profile_cache.cc
@@ -55,16 +55,16 @@
SystemProfileCache::SystemProfileCache()
: initialized_(false),
testing_(false),
- config_root_("/"),
+ metrics_directory_(metrics::kMetricsDirectory),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename)) {
}
SystemProfileCache::SystemProfileCache(bool testing,
- const base::FilePath& config_root)
+ const base::FilePath& metrics_directory)
: initialized_(false),
testing_(testing),
- config_root_(config_root),
+ metrics_directory_(metrics_directory),
session_id_(new chromeos_metrics::PersistentInteger(
kPersistentSessionIdFilename)) {
}
@@ -91,9 +91,11 @@
channel = "";
profile_.version = metrics::kDefaultVersion;
}
- profile_.client_id =
- testing_ ? "client_id_test" :
- GetPersistentGUID(metrics::kMetricsGUIDFilePath);
+ std::string guid_path = metrics_directory_.Append(
+ metrics::kMetricsGUIDFileName).value();
+ profile_.client_id = testing_ ?
+ "client_id_test" :
+ GetPersistentGUID(guid_path);
profile_.hardware_class = "unknown";
profile_.channel = ProtoChannelFromString(channel);
@@ -155,7 +157,7 @@
std::string SystemProfileCache::GetProperty(const std::string& name) {
if (testing_) {
std::string content;
- base::ReadFileToString(config_root_.Append(name), &content);
+ base::ReadFileToString(metrics_directory_.Append(name), &content);
return content;
} else {
char value[PROPERTY_VALUE_MAX];
diff --git a/metricsd/uploader/system_profile_cache.h b/metricsd/uploader/system_profile_cache.h
index 7157810..1d22fa1 100644
--- a/metricsd/uploader/system_profile_cache.h
+++ b/metricsd/uploader/system_profile_cache.h
@@ -50,7 +50,7 @@
public:
SystemProfileCache();
- SystemProfileCache(bool testing, const base::FilePath& config_root);
+ SystemProfileCache(bool testing, const base::FilePath& metrics_directory);
// Populates the ProfileSystem protobuf with system information.
bool Populate(metrics::ChromeUserMetricsExtension* metrics_proto) override;
@@ -77,13 +77,13 @@
bool InitializeOrCheck();
// Gets a system property as a string.
- // When |testing_| is true, reads the value from |config_root_|/|name|
+ // When |testing_| is true, reads the value from |metrics_directory_|/|name|
// instead.
std::string GetProperty(const std::string& name);
bool initialized_;
bool testing_;
- base::FilePath config_root_;
+ base::FilePath metrics_directory_;
scoped_ptr<chromeos_metrics::PersistentInteger> session_id_;
SystemProfile profile_;
};
diff --git a/metricsd/uploader/upload_service.cc b/metricsd/uploader/upload_service.cc
index 2335630..b630cec 100644
--- a/metricsd/uploader/upload_service.cc
+++ b/metricsd/uploader/upload_service.cc
@@ -19,6 +19,7 @@
#include <string>
#include <base/bind.h>
+#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/memory/scoped_vector.h>
#include <base/message_loop/message_loop.h>
@@ -29,6 +30,7 @@
#include <base/metrics/statistics_recorder.h>
#include <base/sha1.h>
+#include "constants.h"
#include "serialization/metric_sample.h"
#include "serialization/serialization_utils.h"
#include "uploader/metrics_log.h"
@@ -44,6 +46,7 @@
metrics_lib_(metrics_lib),
histogram_snapshot_manager_(this),
sender_(new HttpSender(server)),
+ failed_upload_count_(metrics::kFailedUploadCountName),
testing_(false) {
}
@@ -56,9 +59,10 @@
}
void UploadService::Init(const base::TimeDelta& upload_interval,
- const std::string& metrics_file) {
+ const base::FilePath& metrics_directory) {
base::StatisticsRecorder::Initialize();
- metrics_file_ = metrics_file;
+ metrics_file_ = metrics_directory.Append(metrics::kMetricsEventsFileName);
+ staged_log_path_ = metrics_directory.Append(metrics::kStagedLogName);
if (!testing_) {
base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
@@ -70,8 +74,8 @@
}
void UploadService::StartNewLog() {
- CHECK(!staged_log_) << "the staged log should be discarded before starting "
- "a new metrics log";
+ CHECK(!HasStagedLog()) << "the staged log should be discarded before "
+ << "starting a new metrics log";
MetricsLog* log = new MetricsLog();
current_log_.reset(log);
}
@@ -87,7 +91,11 @@
}
void UploadService::UploadEvent() {
- if (staged_log_) {
+ // If the system shutdown or crashed while uploading a report, we may not have
+ // deleted an old log.
+ RemoveFailedLog();
+
+ if (HasStagedLog()) {
// Previous upload failed, retry sending the logs.
SendStagedLog();
return;
@@ -99,51 +107,48 @@
StageCurrentLog();
// If a log is available for upload, upload it.
- if (staged_log_) {
+ if (HasStagedLog()) {
SendStagedLog();
}
}
void UploadService::SendStagedLog() {
- CHECK(staged_log_) << "staged_log_ must exist to be sent";
-
// If metrics are not enabled, discard the log and exit.
if (!metrics_lib_->AreMetricsEnabled()) {
LOG(INFO) << "Metrics disabled. Don't upload metrics samples.";
- staged_log_.reset();
+ base::DeleteFile(staged_log_path_, false);
return;
}
- std::string log_text;
- staged_log_->GetEncodedLog(&log_text);
- if (!sender_->Send(log_text, base::SHA1HashString(log_text))) {
- ++failed_upload_count_;
- if (failed_upload_count_ <= kMaxFailedUpload) {
- LOG(WARNING) << "log upload failed " << failed_upload_count_
- << " times. It will be retried later.";
- return;
- }
- LOG(WARNING) << "log failed more than " << kMaxFailedUpload << " times.";
+ std::string staged_log;
+ CHECK(base::ReadFileToString(staged_log_path_, &staged_log));
+
+ // Increase the failed count in case the daemon crashes while sending the log.
+ failed_upload_count_.Add(1);
+
+ if (!sender_->Send(staged_log, base::SHA1HashString(staged_log))) {
+ LOG(WARNING) << "log failed to upload";
} else {
- LOG(INFO) << "uploaded " << log_text.length() << " bytes";
+ VLOG(1) << "uploaded " << staged_log.length() << " bytes";
+ base::DeleteFile(staged_log_path_, false);
}
- // Discard staged log.
- staged_log_.reset();
+
+ RemoveFailedLog();
}
void UploadService::Reset() {
- staged_log_.reset();
+ base::DeleteFile(staged_log_path_, false);
current_log_.reset();
- failed_upload_count_ = 0;
+ failed_upload_count_.Set(0);
}
void UploadService::ReadMetrics() {
- CHECK(!staged_log_)
- << "cannot read metrics until the old logs have been discarded";
+ CHECK(!HasStagedLog()) << "cannot read metrics until the old logs have been "
+ << "discarded";
ScopedVector<metrics::MetricSample> vector;
metrics::SerializationUtils::ReadAndTruncateMetricsFromFile(
- metrics_file_, &vector);
+ metrics_file_.value(), &vector);
int i = 0;
for (ScopedVector<metrics::MetricSample>::iterator it = vector.begin();
@@ -152,7 +157,7 @@
AddSample(*sample);
i++;
}
- DLOG(INFO) << i << " samples read";
+ VLOG(1) << i << " samples read";
}
void UploadService::AddSample(const metrics::MetricSample& sample) {
@@ -216,19 +221,27 @@
}
void UploadService::StageCurrentLog() {
- CHECK(!staged_log_)
- << "staged logs must be discarded before another log can be staged";
+ // If we haven't logged anything since the last upload, don't upload an empty
+ // report.
+ if (!current_log_)
+ return;
- if (!current_log_) return;
-
- staged_log_.swap(current_log_);
- staged_log_->CloseLog();
- if (!staged_log_->PopulateSystemProfile(system_profile_setter_.get())) {
+ scoped_ptr<MetricsLog> staged_log;
+ staged_log.swap(current_log_);
+ staged_log->CloseLog();
+ if (!staged_log->PopulateSystemProfile(system_profile_setter_.get())) {
LOG(WARNING) << "Error while adding metadata to the log. Discarding the "
<< "log.";
- staged_log_.reset();
+ return;
}
- failed_upload_count_ = 0;
+ std::string encoded_log;
+ staged_log->GetEncodedLog(&encoded_log);
+
+ failed_upload_count_.Set(0);
+ if (static_cast<int>(encoded_log.size()) != base::WriteFile(
+ staged_log_path_, encoded_log.data(), encoded_log.size())) {
+ LOG(ERROR) << "failed to persist to " << staged_log_path_.value();
+ }
}
MetricsLog* UploadService::GetOrCreateCurrentLog() {
@@ -237,3 +250,16 @@
}
return current_log_.get();
}
+
+bool UploadService::HasStagedLog() {
+ return base::PathExists(staged_log_path_);
+}
+
+void UploadService::RemoveFailedLog() {
+ if (failed_upload_count_.Get() > kMaxFailedUpload) {
+ LOG(INFO) << "log failed more than " << kMaxFailedUpload << " times.";
+ CHECK(base::DeleteFile(staged_log_path_, false))
+ << "failed to delete staged log at " << staged_log_path_.value();
+ failed_upload_count_.Set(0);
+ }
+}
diff --git a/metricsd/uploader/upload_service.h b/metricsd/uploader/upload_service.h
index 7f2f413..a4d0a1e 100644
--- a/metricsd/uploader/upload_service.h
+++ b/metricsd/uploader/upload_service.h
@@ -24,6 +24,7 @@
#include "base/metrics/histogram_snapshot_manager.h"
#include "metrics/metrics_library.h"
+#include "persistent_integer.h"
#include "uploader/metrics_log.h"
#include "uploader/sender.h"
#include "uploader/system_profile_cache.h"
@@ -73,7 +74,7 @@
const std::string& server);
void Init(const base::TimeDelta& upload_interval,
- const std::string& metrics_file);
+ const base::FilePath& metrics_directory);
// Starts a new log. The log needs to be regenerated after each successful
// launch as it is destroyed when staging the log.
@@ -106,6 +107,7 @@
FRIEND_TEST(UploadServiceTest, LogContainsAggregatedValues);
FRIEND_TEST(UploadServiceTest, LogEmptyAfterUpload);
FRIEND_TEST(UploadServiceTest, LogEmptyByDefault);
+ FRIEND_TEST(UploadServiceTest, LogFromTheMetricsLibrary);
FRIEND_TEST(UploadServiceTest, LogKernelCrash);
FRIEND_TEST(UploadServiceTest, LogUncleanShutdown);
FRIEND_TEST(UploadServiceTest, LogUserCrash);
@@ -146,6 +148,12 @@
// system information.
void StageCurrentLog();
+ // Returns true iff a log is staged.
+ bool HasStagedLog();
+
+ // Remove the staged log iff the upload failed more than |kMaxFailedUpload|.
+ void RemoveFailedLog();
+
// Returns the current log. If there is no current log, creates it first.
MetricsLog* GetOrCreateCurrentLog();
@@ -153,11 +161,11 @@
MetricsLibraryInterface* metrics_lib_;
base::HistogramSnapshotManager histogram_snapshot_manager_;
scoped_ptr<Sender> sender_;
- int failed_upload_count_;
+ chromeos_metrics::PersistentInteger failed_upload_count_;
scoped_ptr<MetricsLog> current_log_;
- scoped_ptr<MetricsLog> staged_log_;
- std::string metrics_file_;
+ base::FilePath metrics_file_;
+ base::FilePath staged_log_path_;
bool testing_;
};
diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc
index 40c235d..873953e 100644
--- a/metricsd/uploader/upload_service_test.cc
+++ b/metricsd/uploader/upload_service_test.cc
@@ -24,6 +24,7 @@
#include "constants.h"
#include "metrics_library_mock.h"
+#include "persistent_integer.h"
#include "serialization/metric_sample.h"
#include "uploader/metrics_log.h"
#include "uploader/mock/mock_system_profile_setter.h"
@@ -38,17 +39,16 @@
protected:
virtual void SetUp() {
CHECK(dir_.CreateUniqueTempDir());
+ chromeos_metrics::PersistentInteger::SetMetricsDirectory(
+ dir_.path().value());
+ metrics_lib_.InitForTest(dir_.path());
upload_service_.reset(new UploadService(new MockSystemProfileSetter(),
&metrics_lib_, "", true));
upload_service_->sender_.reset(new SenderMock);
- event_file_ = dir_.path().Append("event");
- upload_service_->Init(base::TimeDelta::FromMinutes(30), event_file_.value());
+ upload_service_->Init(base::TimeDelta::FromMinutes(30), dir_.path());
upload_service_->GatherHistograms();
upload_service_->Reset();
-
- chromeos_metrics::PersistentInteger::SetMetricsDirectory(
- dir_.path().value());
}
scoped_ptr<metrics::MetricSample> Crash(const std::string& name) {
@@ -61,11 +61,9 @@
base::WriteFile(dir_.path().Append(name), value.data(), value.size()));
}
- base::FilePath event_file_;
-
base::ScopedTempDir dir_;
scoped_ptr<UploadService> upload_service_;
- MetricsLibraryMock metrics_lib_;
+ MetricsLibrary metrics_lib_;
scoped_ptr<base::AtExitManager> exit_manager_;
};
@@ -135,9 +133,17 @@
upload_service_->UploadEvent();
}
- EXPECT_TRUE(upload_service_->staged_log_);
+ EXPECT_TRUE(upload_service_->HasStagedLog());
upload_service_->UploadEvent();
- EXPECT_FALSE(upload_service_->staged_log_);
+ EXPECT_FALSE(upload_service_->HasStagedLog());
+
+ // Log a new sample. The failed upload counter should be reset.
+ upload_service_->AddSample(*Crash("user"));
+ for (int i = 0; i < UploadService::kMaxFailedUpload; i++) {
+ upload_service_->UploadEvent();
+ }
+ // The log is not discarded after multiple failed uploads.
+ EXPECT_TRUE(upload_service_->HasStagedLog());
}
TEST_F(UploadServiceTest, EmptyLogsAreNotSent) {
@@ -269,3 +275,18 @@
cache.Initialize();
EXPECT_EQ(cache.profile_.session_id, session_id + 1);
}
+
+// Test that we can log metrics from the metrics library and have the uploader
+// upload them.
+TEST_F(UploadServiceTest, LogFromTheMetricsLibrary) {
+ SenderMock* sender = new SenderMock();
+ upload_service_->sender_.reset(sender);
+
+ upload_service_->UploadEvent();
+ EXPECT_EQ(0, sender->send_call_count());
+
+ metrics_lib_.SendEnumToUMA("testname", 2, 10);
+ upload_service_->UploadEvent();
+
+ EXPECT_EQ(1, sender->send_call_count());
+}