Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1184)

Unified Diff: sandbox/linux/services/broker_process_unittest.cc

Issue 229893002: Add unit test to check for broker FD leak (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add BrokerProcessTestHelper to avoid hard-coding macro-generated identifiers Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « sandbox/linux/services/broker_process.h ('k') | sandbox/linux/tests/unit_tests.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sandbox/linux/services/broker_process_unittest.cc
diff --git a/sandbox/linux/services/broker_process_unittest.cc b/sandbox/linux/services/broker_process_unittest.cc
index 7f1a685fe11cbac269e0157e7a0c6825756d3f44..59a347c7b8f1cac3817da19b3317ff72fd522639 100644
--- a/sandbox/linux/services/broker_process_unittest.cc
+++ b/sandbox/linux/services/broker_process_unittest.cc
@@ -11,6 +11,7 @@
#include <sys/wait.h>
#include <unistd.h>
+#include <algorithm>
#include <string>
#include <vector>
@@ -21,12 +22,20 @@
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "base/posix/eintr_wrapper.h"
+#include "base/posix/unix_domain_socket_linux.h"
#include "sandbox/linux/tests/test_utils.h"
#include "sandbox/linux/tests/unit_tests.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace sandbox {
+class BrokerProcessTestHelper {
+ public:
+ static int get_ipc_socketpair(const BrokerProcess* broker) {
+ return broker->ipc_socketpair_;
+ }
+};
+
namespace {
// Creates and open a temporary file on creation and closes
@@ -434,4 +443,53 @@ TEST(BrokerProcess, OpenComplexFlagsNoClientCheck) {
// expected.
}
+// We need to allow noise because the broker will log when it receives our
+// bogus IPCs.
+SANDBOX_TEST_ALLOW_NOISE(BrokerProcess, RecvMsgDescriptorLeak) {
+ // Find the four lowest available file descriptors.
jln (very slow on Chromium) 2014/04/10 20:21:15 Replace with "Find four available file descriptors
mdempsky 2014/04/10 20:35:51 As discussed, POSIX requires pipe() to return the
+ int available_fds[4];
+ SANDBOX_ASSERT(0 == pipe(available_fds));
+ SANDBOX_ASSERT(0 == pipe(available_fds + 2));
+
+ // Save one FD to send to the broker later, and close the others.
+ for (size_t i = 1; i < arraysize(available_fds); i++) {
jln (very slow on Chromium) 2014/04/10 20:21:15 Didn't you forget to close available_fds[0]? Make
mdempsky 2014/04/10 20:35:51 Done.
+ SANDBOX_ASSERT(0 == IGNORE_EINTR(close(available_fds[i])));
+ }
+
+ // Lower our file descriptor limit to just allow three more file descriptors
+ // to be allocated. (N.B., RLIMIT_NOFILE doesn't limit the number of file
+ // descriptors a process can have: it only limits the highest value that can
+ // be assigned to newly-created descriptors allocated by the process.)
+ const rlim_t fd_limit =
jln (very slow on Chromium) 2014/04/10 20:21:15 kFdLimit
mdempsky 2014/04/10 20:35:51 As discussed, this isn't a "compile-time constant"
+ 1 + *std::max_element(available_fds,
+ available_fds + arraysize(available_fds));
+ const struct rlimit new_rlim = {fd_limit, fd_limit};
+ SANDBOX_ASSERT(0 == setrlimit(RLIMIT_NOFILE, &new_rlim));
+
+ const char kCpuInfo[] = "/proc/cpuinfo";
+ std::vector<std::string> read_whitelist;
+ read_whitelist.push_back(kCpuInfo);
+
+ BrokerProcess open_broker(EPERM, read_whitelist, std::vector<std::string>());
+ SANDBOX_ASSERT(open_broker.Init(base::Bind(&NoOpCallback)));
+
+ const int ipc_fd = BrokerProcessTestHelper::get_ipc_socketpair(&open_broker);
+ SANDBOX_ASSERT(ipc_fd >= 0);
+
+ static const char kBogus[] = "not a pickle";
+ std::vector<int> fds;
+ fds.push_back(available_fds[0]);
+
+ // The broker process should only have a couple spare file descriptors
jln (very slow on Chromium) 2014/04/10 20:21:15 Sending kFdLimit descriptors is guaranteed to work
mdempsky 2014/04/10 20:35:51 Yep, that's what I was trying to convey with the c
+ // available, but for good measure we send it fd_limit bogus IPCs anyway.
+ for (rlim_t i = 0; i < fd_limit; ++i) {
+ SANDBOX_ASSERT(
+ UnixDomainSocket::SendMsg(ipc_fd, kBogus, sizeof(kBogus), fds));
+ }
+
+ const int fd = open_broker.Open(kCpuInfo, O_RDONLY);
+ SANDBOX_ASSERT(fd >= 0);
+ SANDBOX_ASSERT(0 == IGNORE_EINTR(close(fd)));
+}
+
} // namespace sandbox
« no previous file with comments | « sandbox/linux/services/broker_process.h ('k') | sandbox/linux/tests/unit_tests.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698