You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2013/08/10 02:46:40 UTC
[1/5] git commit: Improved the ReaperTest by using os::Fork.
Updated Branches:
refs/heads/master c4d96d021 -> 699d70857
Improved the ReaperTest by using os::Fork.
From: Jiang Yan Xu <ya...@jxu.me>
Review: https://reviews.apache.org/r/13389
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b26a133c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b26a133c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b26a133c
Branch: refs/heads/master
Commit: b26a133c599ab342e570ecf050518d2f3b08dea1
Parents: f80698f
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Fri Aug 9 17:22:16 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Fri Aug 9 17:32:23 2013 -0700
----------------------------------------------------------------------
src/tests/reaper_tests.cpp | 108 ++++++++++++++--------------------------
1 file changed, 37 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b26a133c/src/tests/reaper_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/reaper_tests.cpp b/src/tests/reaper_tests.cpp
index d3e1107..608ec0e 100644
--- a/src/tests/reaper_tests.cpp
+++ b/src/tests/reaper_tests.cpp
@@ -34,6 +34,7 @@
#include "slave/reaper.hpp"
+using namespace os;
using namespace mesos;
using namespace mesos::internal;
using namespace mesos::internal::slave;
@@ -48,67 +49,40 @@ using testing::DoDefault;
// This test checks that the Reaper can monitor a non-child process.
TEST(ReaperTest, NonChildProcess)
{
- // Use pipes to determine the pid of the grand child process.
- int pipes[2];
- ASSERT_NE(-1, pipe(pipes));
-
- pid_t pid = fork();
- ASSERT_NE(-1, pid);
-
- if (pid > 0) {
- // In parent process.
- close(pipes[1]);
-
- // Get the grand child's pid via the pipe.
- ASSERT_NE(-1, read(pipes[0], &pid, sizeof(pid)));
-
- close(pipes[0]);
- } else {
- // In child process.
- close(pipes[0]);
-
- // Double fork!
- if ((pid = fork()) == -1) {
- perror("Failed to fork a grand child process");
- abort();
- }
-
- if (pid > 0) {
- // Still in child process.
- exit(0);
- } else {
- // In grand child process.
-
- // Ensure the parent process exits before we write the pid.
- while (getppid() != 1);
-
- pid = getpid();
- if (write(pipes[1], &pid, sizeof(pid)) != sizeof(pid)) {
- perror("Failed to write PID on pipe");
- abort();
- }
- close(pipes[1]);
- while (true); // Keep waiting till we get a signal.
- }
- }
-
- // In parent process.
- LOG(INFO) << "Grand child process " << pid;
+ // The child process creates a grandchild and then exits. The
+ // grandchild sleeps for 10 seconds. The process tree looks like:
+ // -+- child exit 0
+ // \-+- grandchild sleep 10
+
+ // After the child exits, the grandchild is going to be re-parented
+ // by 'init', like this:
+ // -+- child (exit 0)
+ // -+- grandchild sleep 10
+ Try<ProcessTree> tree = Fork(None(),
+ Fork(Exec("sleep 10")),
+ Exec("exit 0"))();
+ ASSERT_SOME(tree);
+ ASSERT_EQ(1u, tree.get().children.size());
+ pid_t grandchild = tree.get().children.front();
Reaper reaper;
Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor);
// Ask the reaper to monitor the grand child process.
- Future<Option<int> > status = reaper.monitor(pid);
+ Future<Option<int> > status = reaper.monitor(grandchild);
AWAIT_READY(monitor);
+ // This makes sure the status only becomes ready after the
+ // grandchild is killed.
+ EXPECT_TRUE(status.isPending());
+
// Now kill the grand child.
// NOTE: We send a SIGKILL here because sometimes the grand child
// process seems to be in a hung state and not responding to
// SIGTERM/SIGINT.
- EXPECT_EQ(0, kill(pid, SIGKILL));
+ EXPECT_EQ(0, kill(grandchild, SIGKILL));
Clock::pause();
@@ -134,28 +108,24 @@ TEST(ReaperTest, ChildProcess)
{
ASSERT_TRUE(GTEST_IS_THREADSAFE);
- pid_t pid = fork();
- ASSERT_NE(-1, pid);
+ // The child process sleeps and will be killed by the parent.
+ Try<ProcessTree> tree = Fork(None(),
+ Exec("sleep 10"))();
- if (pid == 0) {
- // In child process. Keep waiting till we get a signal.
- while (true);
- }
-
- // In parent process.
- LOG(INFO) << "Child process " << pid;
+ ASSERT_SOME(tree);
+ pid_t child = tree.get();
Reaper reaper;
Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor);
// Ask the reaper to monitor the grand child process.
- Future<Option<int> > status = reaper.monitor(pid);
+ Future<Option<int> > status = reaper.monitor(child);
AWAIT_READY(monitor);
// Now kill the child.
- EXPECT_EQ(0, kill(pid, SIGKILL));
+ EXPECT_EQ(0, kill(child, SIGKILL));
Clock::pause();
@@ -184,18 +154,14 @@ TEST(ReaperTest, TerminatedChildProcess)
{
ASSERT_TRUE(GTEST_IS_THREADSAFE);
- pid_t pid = fork();
- ASSERT_NE(-1, pid);
-
- if (pid == 0) {
- // In child process. Return directly
- exit(EXIT_SUCCESS);
- }
+ // The child process immediately exits.
+ Try<ProcessTree> tree = Fork(None(),
+ Exec("exit 0"))();
- // In parent process.
- LOG(INFO) << "Child process " << pid;
+ ASSERT_SOME(tree);
+ pid_t child = tree.get();
- ASSERT_SOME(os::process(pid));
+ ASSERT_SOME(os::process(child));
Clock::pause();
@@ -203,7 +169,7 @@ TEST(ReaperTest, TerminatedChildProcess)
// Because reaper reaps all child processes even if they aren't
// registered, we advance time until that happens.
- while (os::process(pid).isSome()) {
+ while (os::process(child).isSome()) {
Clock::advance(Seconds(1));
Clock::settle();
}
@@ -214,7 +180,7 @@ TEST(ReaperTest, TerminatedChildProcess)
Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor);
// Ask the reaper to monitor the child process.
- Future<Option<int> > status = reaper.monitor(pid);
+ Future<Option<int> > status = reaper.monitor(child);
AWAIT_READY(monitor);
[5/5] git commit: Fixed a performance issue in Master::Http::stats().
Posted by bm...@apache.org.
Fixed a performance issue in Master::Http::stats().
We spent a lot of time doing unnecessary range additions
(e.g. ports resources).
From: Jie Yu <yu...@gmail.com>
Review: https://reviews.apache.org/r/13433
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/699d7085
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/699d7085
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/699d7085
Branch: refs/heads/master
Commit: 699d708573737ea0225134bdcaf58f064eda3711
Parents: b26a133
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Fri Aug 9 17:22:49 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Fri Aug 9 17:32:24 2013 -0700
----------------------------------------------------------------------
src/master/http.cpp | 36 +++++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/699d7085/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index ca66d67..1ac84a9 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -312,22 +312,32 @@ Future<Response> Master::Http::stats(const Request& request)
Resources totalResources;
Resources usedResources;
foreachvalue (Slave* slave, master.slaves) {
- totalResources += slave->info.resources();
- usedResources += slave->resourcesInUse;
+ // Instead of accumulating all types of resources (which is
+ // not necessary), we only accumulate scalar resources. This
+ // helps us bypass a performance problem caused by range
+ // additions (e.g. ports).
+ foreach (const Resource& resource, slave->info.resources()) {
+ if (resource.type() == Value::SCALAR) {
+ totalResources += resource;
+ }
+ }
+ foreach (const Resource& resource, slave->resourcesInUse) {
+ if (resource.type() == Value::SCALAR) {
+ usedResources += resource;
+ }
+ }
}
foreach (const Resource& resource, totalResources) {
- if (resource.type() == Value::SCALAR) {
- CHECK(resource.has_scalar());
- double total = resource.scalar().value();
- object.values[resource.name() + "_total"] = total;
- Option<Resource> option = usedResources.get(resource);
- CHECK(!option.isSome() || option.get().has_scalar());
- double used = option.isSome() ? option.get().scalar().value() : 0.0;
- object.values[resource.name() + "_used"] = used;
- double percent = used / total;
- object.values[resource.name() + "_percent"] = percent;
- }
+ CHECK(resource.has_scalar());
+ double total = resource.scalar().value();
+ object.values[resource.name() + "_total"] = total;
+ Option<Resource> option = usedResources.get(resource);
+ CHECK(!option.isSome() || option.get().has_scalar());
+ double used = option.isSome() ? option.get().scalar().value() : 0.0;
+ object.values[resource.name() + "_used"] = used;
+ double percent = used / total;
+ object.values[resource.name() + "_percent"] = percent;
}
return OK(object, request.query.get("jsonp"));
[2/5] git commit: Changed proc::status(pid) to use os::read().
Posted by bm...@apache.org.
Changed proc::status(pid) to use os::read().
This was changed because ifstream threw unexpected exceptions.
From: Jiang Yan Xu <ya...@jxu.me>
Review: https://reviews.apache.org/r/13388
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/f80698f0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/f80698f0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/f80698f0
Branch: refs/heads/master
Commit: f80698f00e7ebd3c7ced5b4704f3fd3c6c7093f4
Parents: 4d2b9f2
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Fri Aug 9 17:21:03 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Fri Aug 9 17:32:23 2013 -0700
----------------------------------------------------------------------
.../3rdparty/stout/include/stout/proc.hpp | 22 +++++++++-----------
1 file changed, 10 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/f80698f0/3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp
index d51aaa9..c3eac30 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/proc.hpp
@@ -15,7 +15,7 @@
#include <list>
#include <queue>
#include <set>
-#include <sstream> // For 'std::stringbuf'.
+#include <sstream> // For 'std::istringstream'.
#include <string>
#include <vector>
@@ -29,6 +29,7 @@
#include <stout/os/exists.hpp>
#include <stout/os/ls.hpp>
+#include <stout/os/read.hpp>
namespace proc {
@@ -153,18 +154,18 @@ inline Result<ProcessStatus> status(pid_t pid)
{
std::string path = "/proc/" + stringify(pid) + "/stat";
- std::ifstream file(path.c_str());
-
- if (!file.is_open()) {
+ Try<std::string> read = os::read(path);
+ if (read.isError()) {
// Need to check if file exists AFTER we open it to guarantee
- // process hasn't terminated (or if it has, we at least have a
- // file which the kernel _should_ respect until a close).
+ // process hasn't terminated.
if (!os::exists(path)) {
return None();
}
- return Error("Failed to open '" + path + "'");
+ return Error(read.error());
}
+ std::istringstream data(read.get());
+
std::string comm;
char state;
pid_t ppid;
@@ -212,7 +213,7 @@ inline Result<ProcessStatus> status(pid_t pid)
std::string _; // For ignoring fields.
// Parse all fields from stat.
- file >> _ >> comm >> state >> ppid >> pgrp >> session >> tty_nr
+ data >> _ >> comm >> state >> ppid >> pgrp >> session >> tty_nr
>> tpgid >> flags >> minflt >> cminflt >> majflt >> cmajflt
>> utime >> stime >> cutime >> cstime >> priority >> nice
>> num_threads >> itrealvalue >> starttime >> vsize >> rss
@@ -220,8 +221,7 @@ inline Result<ProcessStatus> status(pid_t pid)
>> signal >> blocked >> sigcatch >> wchan >> nswap >> cnswap;
// Check for any read/parse errors.
- if (file.fail() && !file.eof()) {
- file.close();
+ if (data.fail() && !data.eof()) {
return Error("Failed to read/parse '" + path + "'");
}
@@ -231,8 +231,6 @@ inline Result<ProcessStatus> status(pid_t pid)
comm = strings::remove(comm, "(", strings::PREFIX);
comm = strings::remove(comm, ")", strings::SUFFIX);
- file.close();
-
return ProcessStatus(pid, comm, state, ppid, pgrp, session, tty_nr,
tpgid, flags, minflt, cminflt, majflt, cmajflt,
utime, stime, cutime, cstime, priority, nice,
[3/5] git commit: Fixed session handling on OS X.
Posted by bm...@apache.org.
Fixed session handling on OS X.
On OS X, it appears that getsid() returns ESRCH for zombie processes.
Review: https://reviews.apache.org/r/13396
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5d0195b3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5d0195b3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5d0195b3
Branch: refs/heads/master
Commit: 5d0195b3c6161e2cb65b39a2d43192946ae7d767
Parents: c4d96d0
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Wed Aug 7 17:06:02 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Fri Aug 9 17:32:23 2013 -0700
----------------------------------------------------------------------
.../libprocess/3rdparty/stout/include/stout/os.hpp | 7 +++++--
.../3rdparty/stout/include/stout/os/killtree.hpp | 15 ++++++++++-----
.../3rdparty/stout/include/stout/os/osx.hpp | 8 ++++++--
.../3rdparty/stout/include/stout/os/process.hpp | 4 ++--
.../libprocess/3rdparty/stout/tests/os_tests.cpp | 6 ++++--
5 files changed, 27 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/5d0195b3/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
index 448739e..5599b44 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
@@ -1110,12 +1110,15 @@ inline Try<std::set<pid_t> > pids(Option<pid_t> group, Option<pid_t> session)
foreach (const Process& process, processes.get()) {
// Group AND Session (intersection).
if (group.isSome() && session.isSome()) {
- if (group.get() == process.group && session.get() == process.session) {
+ if (group.get() == process.group &&
+ process.session.isSome() &&
+ session.get() == process.session.get()) {
result.insert(process.pid);
}
} else if (group.isSome() && group.get() == process.group) {
result.insert(process.pid);
- } else if (session.isSome() && session.get() == process.session) {
+ } else if (session.isSome() && process.session.isSome() &&
+ session.get() == process.session.get()) {
result.insert(process.pid);
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/5d0195b3/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
index 9dc314c..25e9937 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
@@ -71,8 +71,8 @@ inline Try<std::list<ProcessTree> > killtree(
if (groups) {
visited.groups.insert(parent.get().group);
}
- if (sessions) {
- visited.sessions.insert(parent.get().session);
+ if (sessions && parent.get().session.isSome()) {
+ visited.sessions.insert(parent.get().session.get());
}
}
}
@@ -131,11 +131,16 @@ inline Try<std::list<ProcessTree> > killtree(
}
}
- if (sessions) {
- pid_t session = process.get().session;
+ // If we do not have a session for the process, it's likely
+ // because the process is a zombie on OS X. This implies it has
+ // not been reaped and thus is located somewhere in the tree we
+ // are trying to kill. Therefore, we should discover it from our
+ // tree traversal, or through its group (which is always present).
+ if (sessions && process.get().session.isSome()) {
+ pid_t session = process.get().session.get();
if (visited.sessions.count(session) == 0) {
foreach (const Process& process, processes.get()) {
- if (process.session == session) {
+ if (process.session.isSome() && process.session.get() == session) {
queue.push(process.pid);
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/5d0195b3/3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp
index 4161e77..7d02566 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp
@@ -110,11 +110,15 @@ inline Result<Process> process(pid_t pid)
proc_taskinfo task;
int size = proc_pidinfo(pid, PROC_PIDTASKINFO, 0, &task, sizeof(task));
+ // It appears that zombie processes on OS X do not have sessions and
+ // result in ESRCH.
+ int session = getsid(pid);
+
if (size != sizeof(task)) {
return Process(process.kp_proc.p_pid,
process.kp_eproc.e_ppid,
process.kp_eproc.e_pgid,
- getsid(pid),
+ session > 0 ? session : Option<pid_t>::none(),
None(),
None(),
None(),
@@ -124,7 +128,7 @@ inline Result<Process> process(pid_t pid)
return Process(process.kp_proc.p_pid,
process.kp_eproc.e_ppid,
process.kp_eproc.e_pgid,
- getsid(pid),
+ session > 0 ? session : Option<pid_t>::none(),
Bytes(task.pti_resident_size),
Nanoseconds(task.pti_total_user),
Nanoseconds(task.pti_total_system),
http://git-wip-us.apache.org/repos/asf/mesos/blob/5d0195b3/3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp
index 9dbb89f..d754601 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/process.hpp
@@ -21,7 +21,7 @@ struct Process
Process(pid_t _pid,
pid_t _parent,
pid_t _group,
- pid_t _session,
+ const Option<pid_t>& _session,
const Option<Bytes>& _rss,
const Option<Duration>& _utime,
const Option<Duration>& _stime,
@@ -40,7 +40,7 @@ struct Process
const pid_t pid;
const pid_t parent;
const pid_t group;
- const pid_t session;
+ const Option<pid_t> session;
const Option<Bytes> rss;
const Option<Duration> utime;
const Option<Duration> stime;
http://git-wip-us.apache.org/repos/asf/mesos/blob/5d0195b3/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
index f926056..615028d 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -340,7 +340,8 @@ TEST_F(OsTest, process)
ASSERT_SOME(process);
EXPECT_EQ(getpid(), process.get().pid);
EXPECT_EQ(getppid(), process.get().parent);
- EXPECT_EQ(getsid(getpid()), process.get().session);
+ ASSERT_SOME(process.get().session);
+ EXPECT_EQ(getsid(getpid()), process.get().session.get());
ASSERT_SOME(process.get().rss);
EXPECT_GT(process.get().rss.get(), 0);
@@ -370,7 +371,8 @@ TEST_F(OsTest, processes)
found = true;
EXPECT_EQ(getpid(), process.pid);
EXPECT_EQ(getppid(), process.parent);
- EXPECT_EQ(getsid(getpid()), process.session);
+ ASSERT_SOME(process.session);
+ EXPECT_EQ(getsid(getpid()), process.session.get());
ASSERT_SOME(process.rss);
EXPECT_GT(process.rss.get(), 0);
[4/5] git commit: Rewrote os::read(string path) to use getline.
Posted by bm...@apache.org.
Rewrote os::read(string path) to use getline.
The existing implemenation does not work well when used to read
files in the /proc pseudo filesystem.
From: Jiang Yan Xu <ya...@jxu.me>
Review: https://reviews.apache.org/r/13387
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4d2b9f28
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4d2b9f28
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4d2b9f28
Branch: refs/heads/master
Commit: 4d2b9f28c1da6bf908e2859d1cee281abd58e7b1
Parents: 5d0195b
Author: Benjamin Mahler <bm...@twitter.com>
Authored: Fri Aug 9 17:17:20 2013 -0700
Committer: Benjamin Mahler <bm...@twitter.com>
Committed: Fri Aug 9 17:32:23 2013 -0700
----------------------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/Makefile.am | 1 +
.../3rdparty/stout/include/stout/os.hpp | 96 +-------------------
.../3rdparty/stout/include/stout/os/read.hpp | 93 +++++++++++++++++++
.../3rdparty/stout/tests/os_tests.cpp | 2 +-
4 files changed, 97 insertions(+), 95 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/4d2b9f28/3rdparty/libprocess/3rdparty/stout/Makefile.am
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/Makefile.am b/3rdparty/libprocess/3rdparty/stout/Makefile.am
index 770378d..e465fd1 100644
--- a/3rdparty/libprocess/3rdparty/stout/Makefile.am
+++ b/3rdparty/libprocess/3rdparty/stout/Makefile.am
@@ -41,6 +41,7 @@ EXTRA_DIST = \
include/stout/os/ls.hpp \
include/stout/os/osx.hpp \
include/stout/os/process.hpp \
+ include/stout/os/read.hpp \
include/stout/os/sendfile.hpp \
include/stout/os/signals.hpp \
include/stout/os/pstree.hpp \
http://git-wip-us.apache.org/repos/asf/mesos/blob/4d2b9f28/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
index 5599b44..728122e 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
@@ -60,12 +60,13 @@
#ifdef __APPLE__
#include <stout/os/osx.hpp>
#endif // __APPLE__
+#include <stout/os/pstree.hpp>
+#include <stout/os/read.hpp>
#include <stout/os/sendfile.hpp>
#include <stout/os/signals.hpp>
#ifdef __APPLE__
#include <stout/os/sysctl.hpp>
#endif // __APPLE__
-#include <stout/os/pstree.hpp>
#ifdef __APPLE__
// Assigning the result pointer to ret silences an unused var warning.
@@ -306,99 +307,6 @@ inline Try<Nothing> write(const std::string& path, const std::string& message)
}
-// Reads 'size' bytes from a file from its current offset.
-// If EOF is encountered before reading size bytes, then the offset
-// is restored and none is returned.
-inline Result<std::string> read(int fd, size_t size)
-{
- // Save the current offset.
- off_t current = lseek(fd, 0, SEEK_CUR);
- if (current == -1) {
- return ErrnoError("Failed to lseek to SEEK_CUR");
- }
-
- char* buffer = new char[size];
- size_t offset = 0;
-
- while (offset < size) {
- ssize_t length = ::read(fd, buffer + offset, size - offset);
-
- if (length < 0) {
- // TODO(bmahler): Handle a non-blocking fd? (EAGAIN, EWOULDBLOCK)
- if (errno == EINTR) {
- continue;
- }
- // Attempt to restore the original offset.
- lseek(fd, current, SEEK_SET);
- return ErrnoError();
- } else if (length == 0) {
- // Reached EOF before expected! Restore the offset.
- lseek(fd, current, SEEK_SET);
- return None();
- }
-
- offset += length;
- }
-
- return std::string(buffer, size);
-}
-
-
-// Returns the contents of the file starting from its current offset.
-// If an error occurs, this will attempt to recover the file offset.
-inline Try<std::string> read(int fd)
-{
- // Save the current offset.
- off_t current = lseek(fd, 0, SEEK_CUR);
- if (current == -1) {
- return ErrnoError("Failed to lseek to SEEK_CUR");
- }
-
- // Get the size of the file from the offset.
- off_t size = lseek(fd, current, SEEK_END);
- if (size == -1) {
- return ErrnoError("Failed to lseek to SEEK_END");
- }
-
- // Restore the offset.
- if (lseek(fd, current, SEEK_SET) == -1) {
- return ErrnoError("Failed to lseek with SEEK_SET");
- }
-
- Result<std::string> result = read(fd, size);
- if (result.isNone()) {
- // Hit EOF before reading size bytes.
- return Error("The file size was modified while reading");
- } else if (result.isError()) {
- return Error(result.error());
- }
-
- return result.get();
-}
-
-
-// A wrapper function that wraps the above read() with
-// open and closing the file.
-inline Try<std::string> read(const std::string& path)
-{
- Try<int> fd =
- os::open(path, O_RDONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO);
-
- if (fd.isError()) {
- return Error("Failed to open file '" + path + "'");
- }
-
- Try<std::string> result = read(fd.get());
-
- // NOTE: We ignore the return value of close(). This is because users calling
- // this function are interested in the return value of read(). Also an
- // unsuccessful close() doesn't affect the read.
- os::close(fd.get());
-
- return result;
-}
-
-
inline Try<Nothing> rm(const std::string& path)
{
if (::remove(path.c_str()) != 0) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/4d2b9f28/3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp
new file mode 100644
index 0000000..0c5b2ef
--- /dev/null
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp
@@ -0,0 +1,93 @@
+#ifndef __STOUT_OS_READ_HPP__
+#define __STOUT_OS_READ_HPP__
+
+#include <stdio.h>
+#include <unistd.h>
+
+#include <stout/error.hpp>
+#include <stout/try.hpp>
+
+namespace os {
+
+// Reads 'size' bytes from a file from its current offset.
+// If EOF is encountered before reading size bytes, then the offset
+// is restored and none is returned.
+inline Result<std::string> read(int fd, size_t size)
+{
+ // Save the current offset.
+ off_t current = lseek(fd, 0, SEEK_CUR);
+ if (current == -1) {
+ return ErrnoError("Failed to lseek to SEEK_CUR");
+ }
+
+ char* buffer = new char[size];
+ size_t offset = 0;
+
+ while (offset < size) {
+ ssize_t length = ::read(fd, buffer + offset, size - offset);
+
+ if (length < 0) {
+ // TODO(bmahler): Handle a non-blocking fd? (EAGAIN, EWOULDBLOCK)
+ if (errno == EINTR) {
+ continue;
+ }
+ // Attempt to restore the original offset.
+ lseek(fd, current, SEEK_SET);
+ return ErrnoError();
+ } else if (length == 0) {
+ // Reached EOF before expected! Restore the offset.
+ lseek(fd, current, SEEK_SET);
+ return None();
+ }
+
+ offset += length;
+ }
+
+ return std::string(buffer, size);
+}
+
+
+// Returns the contents of the file.
+inline Try<std::string> read(const std::string& path)
+{
+ FILE* file = fopen(path.c_str(), "r");
+ if (file == NULL) {
+ return ErrnoError("Failed to open file '" + path + "'");
+ }
+
+ // Initially the 'line' is NULL and length 0, getline() allocates
+ // ('malloc') a buffer for reading the line.
+ // In subsequent iterations, if the buffer is not large enough to
+ // hold the line, getline() resizes it with 'realloc' and updates
+ // 'line' and 'length' as necessary. See:
+ // - http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html
+ // - http://man7.org/linux/man-pages/man3/getline.3.html
+ std::string result;
+ char* line = NULL;
+ size_t length = 0;
+ ssize_t read;
+
+ while ((read = getline(&line, &length, file)) != -1) {
+ result.append(line, read);
+ }
+
+ // getline() requires the line buffer to be freed by the caller.
+ free(line);
+
+ if (ferror(file)) {
+ ErrnoError error;
+ // NOTE: We ignore the error from fclose(). This is because
+ // users calling this function are interested in the return value
+ // of read(). Also an unsuccessful fclose() does not affect the
+ // read.
+ fclose(file);
+ return error;
+ }
+
+ fclose(file);
+ return result;
+}
+
+} // namespace os {
+
+#endif // __STOUT_OS_READ_HPP__
http://git-wip-us.apache.org/repos/asf/mesos/blob/4d2b9f28/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
index 615028d..0ef2eb5 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -133,7 +133,7 @@ TEST_F(OsTest, touch)
TEST_F(OsTest, readWriteString)
{
const string& testfile = tmpdir + "/" + UUID::random().toString();
- const string& teststr = "test";
+ const string& teststr = "line1\nline2";
ASSERT_SOME(os::write(testfile, teststr));