diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index b4c6a647d70d6626ada1e01c2f39b89a50af2d8b..3b17b819bd7c58caae53de89ddd8dad7e773b702 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -47,11 +47,18 @@
#include <typeinfo>
#include <utility>
-static const char* whichfile[] = { "stdin", "stdout", "stderr" };
+static const char* whichfile_[] = { "stdin", "stdout", "stderr" };
static std::string empty;
static LLProcess::Status interpret_status(int status);
static std::string getDesc(const LLProcess::Params& params);
+static std::string whichfile(LLProcess::FILESLOT index)
+{
+ if (index < LL_ARRAY_SIZE(whichfile_))
+ return whichfile_[index];
+ return STRINGIZE("file slot " << index);
+}
+
/**
* Ref-counted "mainloop" listener. As long as there are still outstanding
* LLProcess objects, keep listening on "mainloop" so we can keep polling APR
@@ -122,6 +129,7 @@ static LLProcessListener sProcessListener;
LLProcess::BasePipe::~BasePipe() {}
const LLProcess::BasePipe::size_type
+ // use funky syntax to call max() to avoid blighted max() macros
LLProcess::BasePipe::npos((std::numeric_limits<LLProcess::BasePipe::size_type>::max)());
class WritePipeImpl: public LLProcess::WritePipe
@@ -205,14 +213,14 @@ class ReadPipeImpl: public LLProcess::ReadPipe
{
LOG_CLASS(ReadPipeImpl);
public:
- ReadPipeImpl(const std::string& desc, apr_file_t* pipe):
+ ReadPipeImpl(const std::string& desc, apr_file_t* pipe, LLProcess::FILESLOT index):
mDesc(desc),
mPipe(pipe),
+ mIndex(index),
// Essential to initialize our std::istream with our special streambuf!
mStream(&mStreambuf),
mPump("ReadPipe", true), // tweak name as needed to avoid collisions
- // use funky syntax to call max() to avoid blighted max() macros
- mLimit(npos)
+ mLimit(0)
{
mConnection = LLEventPumps::instance().obtain("mainloop")
.listen(LLEventPump::inventName("ReadPipe"),
@@ -364,7 +372,10 @@ class ReadPipeImpl: public LLProcess::ReadPipe
size_type datasize((std::min)(mLimit, size_type(mStreambuf.size())));
mPump.post(LLSDMap
("data", peek(0, datasize))
- ("len", LLSD::Integer(mStreambuf.size())));
+ ("len", LLSD::Integer(mStreambuf.size()))
+ ("index", LLSD::Integer(mIndex))
+ ("name", whichfile(mIndex))
+ ("desc", mDesc));
}
return false;
@@ -372,6 +383,7 @@ class ReadPipeImpl: public LLProcess::ReadPipe
std::string mDesc;
apr_file_t* mPipe;
+ LLProcess::FILESLOT mIndex;
LLTempBoundListener mConnection;
boost::asio::streambuf mStreambuf;
std::istream mStream;
@@ -400,7 +412,7 @@ LLProcessPtr LLProcess::create(const LLSDOrParams& params)
// indicating bad launch. This may prevent someone waiting forever for
// a termination post that can't arrive because the child never
// started.
- if (! std::string(params.postend).empty())
+ if (params.postend.isProvided())
{
LLEventPumps::instance().obtain(params.postend)
.post(LLSDMap
@@ -458,22 +470,24 @@ LLProcess::LLProcess(const LLSDOrParams& params):
// and passing it as both stdout and stderr (apr_procattr_child_out_set(),
// apr_procattr_child_err_set()), or accepting a filename, opening it and
// passing that apr_file_t (simple <, >, 2> redirect emulation).
- std::vector<FileParam> fparams(params.files.begin(), params.files.end());
- // By default, pass APR_NO_PIPE for each slot.
- std::vector<apr_int32_t> select(LL_ARRAY_SIZE(whichfile), APR_NO_PIPE);
- for (size_t i = 0; i < (std::min)(LL_ARRAY_SIZE(whichfile), fparams.size()); ++i)
+ std::vector<apr_int32_t> select;
+ BOOST_FOREACH(const FileParam& fparam, params.files)
{
- if (std::string(fparams[i].type).empty()) // inherit our file descriptor
+ // Every iteration, we're going to append an item to 'select'. At the
+ // top of the loop, its size() is, in effect, an index. Use that to
+ // pick a string description for messages.
+ std::string which(whichfile(FILESLOT(select.size())));
+ if (fparam.type().empty()) // inherit our file descriptor
{
- select[i] = APR_NO_PIPE;
+ select.push_back(APR_NO_PIPE);
}
- else if (std::string(fparams[i].type) == "pipe") // anonymous pipe
+ else if (fparam.type() == "pipe") // anonymous pipe
{
- if (! std::string(fparams[i].name).empty())
+ if (! fparam.name().empty())
{
- LL_WARNS("LLProcess") << "For " << std::string(params.executable)
+ LL_WARNS("LLProcess") << "For " << params.executable()
<< ": internal names for reusing pipes ('"
- << std::string(fparams[i].name) << "' for " << whichfile[i]
+ << fparam.name() << "' for " << which
<< ") are not yet supported -- creating distinct pipe"
<< LL_ENDL;
}
@@ -482,16 +496,21 @@ LLProcess::LLProcess(const LLSDOrParams& params):
// makes very little sense to set nonblocking I/O for the child
// end of a pipe: only a specially-written child could deal with
// that.
- select[i] = APR_CHILD_BLOCK;
+ select.push_back(APR_CHILD_BLOCK);
}
else
{
- throw LLProcessError(STRINGIZE("For " << std::string(params.executable)
- << ": unsupported FileParam for " << whichfile[i]
- << ": type='" << std::string(fparams[i].type)
- << "', name='" << std::string(fparams[i].name) << "'"));
+ throw LLProcessError(STRINGIZE("For " << params.executable()
+ << ": unsupported FileParam for " << which
+ << ": type='" << fparam.type()
+ << "', name='" << fparam.name() << "'"));
}
}
+ // By default, pass APR_NO_PIPE for unspecified slots.
+ while (select.size() < NSLOTS)
+ {
+ select.push_back(APR_NO_PIPE);
+ }
chkapr(apr_procattr_io_set(procattr, select[STDIN], select[STDOUT], select[STDERR]));
// Thumbs down on implicitly invoking the shell to invoke the child. From
@@ -527,24 +546,32 @@ LLProcess::LLProcess(const LLSDOrParams& params):
#endif
}
- // Have to instantiate named std::strings for string params items so their
- // c_str() values persist.
- std::string cwd(params.cwd);
- if (! cwd.empty())
+ // In preparation for calling apr_proc_create(), we collect a number of
+ // const char* pointers obtained from std::string::c_str(). Turns out
+ // LLInitParam::Block's helpers Optional, Mandatory, Multiple et al.
+ // guarantee that converting to the wrapped type (std::string in our
+ // case), e.g. by calling operator(), returns a reference to *the same
+ // instance* of the wrapped type that's stored in our Block subclass.
+ // That's important! We know 'params' persists throughout this method
+ // call; but without that guarantee, we'd have to assume that converting
+ // one of its members to std::string might return a different (temp)
+ // instance. Capturing the c_str() from a temporary std::string is Bad Bad
+ // Bad. But armed with this knowledge, when you see params.cwd().c_str(),
+ // grit your teeth and smile and carry on.
+
+ if (params.cwd.isProvided())
{
- chkapr(apr_procattr_dir_set(procattr, cwd.c_str()));
+ chkapr(apr_procattr_dir_set(procattr, params.cwd().c_str()));
}
// create an argv vector for the child process
std::vector<const char*> argv;
- // add the executable path
- std::string executable(params.executable);
- argv.push_back(executable.c_str());
+ // Add the executable path. See above remarks about c_str().
+ argv.push_back(params.executable().c_str());
- // and any arguments
- std::vector<std::string> args(params.args.begin(), params.args.end());
- BOOST_FOREACH(const std::string& arg, args)
+ // Add arguments. See above remarks about c_str().
+ BOOST_FOREACH(const std::string& arg, params.args)
{
argv.push_back(arg.c_str());
}
@@ -590,7 +617,7 @@ LLProcess::LLProcess(const LLSDOrParams& params):
{
if (select[i] != APR_CHILD_BLOCK)
continue;
- std::string desc(STRINGIZE(mDesc << ' ' << whichfile[i]));
+ std::string desc(STRINGIZE(mDesc << ' ' << whichfile(FILESLOT(i))));
apr_file_t* pipe(mProcess.*(members[i]));
if (i == STDIN)
{
@@ -598,7 +625,7 @@ LLProcess::LLProcess(const LLSDOrParams& params):
}
else
{
- mPipes.replace(i, new ReadPipeImpl(desc, pipe));
+ mPipes.replace(i, new ReadPipeImpl(desc, pipe, FILESLOT(i)));
}
LL_DEBUGS("LLProcess") << "Instantiating " << typeid(mPipes[i]).name()
<< "('" << desc << "')" << LL_ENDL;
@@ -609,9 +636,8 @@ LLProcess::LLProcess(const LLSDOrParams& params):
static std::string getDesc(const LLProcess::Params& params)
{
// If caller specified a description string, by all means use it.
- std::string desc(params.desc);
- if (! desc.empty())
- return desc;
+ if (params.desc.isProvided())
+ return params.desc;
// Caller didn't say. Use the executable name -- but use just the filename
// part. On Mac, for instance, full pathnames get cumbersome.
@@ -670,22 +696,22 @@ bool LLProcess::kill(const std::string& who)
return ! isRunning();
}
-bool LLProcess::isRunning(void)
+bool LLProcess::isRunning() const
{
return getStatus().mState == RUNNING;
}
-LLProcess::Status LLProcess::getStatus()
+LLProcess::Status LLProcess::getStatus() const
{
return mStatus;
}
-std::string LLProcess::getStatusString()
+std::string LLProcess::getStatusString() const
{
return getStatusString(getStatus());
}
-std::string LLProcess::getStatusString(const Status& status)
+std::string LLProcess::getStatusString(const Status& status) const
{
return getStatusString(mDesc, status);
}
@@ -816,7 +842,7 @@ LLProcess::handle LLProcess::getProcessHandle() const
#endif
}
-std::string LLProcess::getPipeName(FILESLOT)
+std::string LLProcess::getPipeName(FILESLOT) const
{
// LLProcess::FileParam::type "npipe" is not yet implemented
return "";
@@ -832,7 +858,7 @@ PIPETYPE* LLProcess::getPipePtr(std::string& error, FILESLOT slot)
}
if (mPipes.is_null(slot))
{
- error = STRINGIZE(mDesc << ' ' << whichfile[slot] << " not a monitored pipe");
+ error = STRINGIZE(mDesc << ' ' << whichfile(slot) << " not a monitored pipe");
return NULL;
}
// Make sure we dynamic_cast in pointer domain so we can test, rather than
@@ -840,7 +866,7 @@ PIPETYPE* LLProcess::getPipePtr(std::string& error, FILESLOT slot)
PIPETYPE* ppipe = dynamic_cast<PIPETYPE*>(&mPipes[slot]);
if (! ppipe)
{
- error = STRINGIZE(mDesc << ' ' << whichfile[slot] << " not a " << typeid(PIPETYPE).name());
+ error = STRINGIZE(mDesc << ' ' << whichfile(slot) << " not a " << typeid(PIPETYPE).name());
return NULL;
}
@@ -895,10 +921,9 @@ boost::optional<LLProcess::ReadPipe&> LLProcess::getOptReadPipe(FILESLOT slot)
std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params)
{
- std::string cwd(params.cwd);
- if (! cwd.empty())
+ if (params.cwd.isProvided())
{
- out << "cd " << LLStringUtil::quote(cwd) << ": ";
+ out << "cd " << LLStringUtil::quote(params.cwd) << ": ";
}
out << LLStringUtil::quote(params.executable);
BOOST_FOREACH(const std::string& arg, params.args)
diff --git a/indra/llcommon/llprocess.h b/indra/llcommon/llprocess.h
index d005847e18ea95f307f130d829387a33ca61c72a..637b7e2f9c8167f1c1ef7acac737e35434067621 100644
--- a/indra/llcommon/llprocess.h
+++ b/indra/llcommon/llprocess.h
@@ -58,12 +58,16 @@ typedef boost::shared_ptr<LLProcess> LLProcessPtr;
* arguments. It also keeps track of whether the process is still running, and
* can kill it if required.
*
+ * In discussing LLProcess, we use the term "parent" to refer to this process
+ * (the process invoking LLProcess), versus "child" to refer to the process
+ * spawned by LLProcess.
+ *
* LLProcess relies on periodic post() calls on the "mainloop" LLEventPump: an
- * LLProcess object's Status won't update until the next "mainloop" tick. The
- * viewer's main loop already posts to that LLEventPump once per iteration
- * (hence the name). See indra/llcommon/tests/llprocess_test.cpp for an
- * example of waiting for child-process termination in a standalone test
- * context.
+ * LLProcess object's Status won't update until the next "mainloop" tick. For
+ * instance, the Second Life viewer's main loop already posts to an
+ * LLEventPump by that name once per iteration. See
+ * indra/llcommon/tests/llprocess_test.cpp for an example of waiting for
+ * child-process termination in a standalone test context.
*/
class LL_COMMON_API LLProcess: public boost::noncopyable
{
@@ -110,10 +114,10 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
* pumping nonblocking I/O every frame. The expectation (at least
* for stdout or stderr) is that the caller will listen for
* incoming data and consume it as it arrives. It's important not
- * to neglect such a pipe, because it's buffered in viewer memory.
- * If you suspect the child may produce a great volume of output
- * between viewer frames, consider directing the child to write to
- * a filesystem file instead, then read the file later.
+ * to neglect such a pipe, because it's buffered in memory. If you
+ * suspect the child may produce a great volume of output between
+ * frames, consider directing the child to write to a filesystem
+ * file instead, then read the file later.
*
* - "tpipe": do not engage LLProcess machinery to monitor the
* parent end of the pipe. A "tpipe" is used only to connect
@@ -145,9 +149,14 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
Optional<std::string> name;
FileParam(const std::string& tp="", const std::string& nm=""):
- type("type", tp),
- name("name", nm)
- {}
+ type("type"),
+ name("name")
+ {
+ // If caller wants to specify values, use explicit assignment to
+ // set them rather than initialization.
+ if (! tp.empty()) type = tp;
+ if (! nm.empty()) name = nm;
+ }
};
/// Param block definition
@@ -175,17 +184,28 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
/// current working directory, if need it changed
Optional<std::string> cwd;
/// implicitly kill process on destruction of LLProcess object
+ /// (default true)
Optional<bool> autokill;
/**
* Up to three FileParam items: for child stdin, stdout, stderr.
* Passing two FileParam entries means default treatment for stderr,
* and so forth.
*
+ * @note LLInitParam::Block permits usage like this:
+ * @code
+ * LLProcess::Params params;
+ * ...
+ * params.files
+ * .add(LLProcess::FileParam()) // stdin
+ * .add(LLProcess::FileParam().type("pipe") // stdout
+ * .add(LLProcess::FileParam().type("file").name("error.log"));
+ * @endcode
+ *
* @note While it's theoretically plausible to pass additional open
* file handles to a child specifically written to expect them, our
- * underlying implementation library doesn't support that.
+ * underlying implementation doesn't yet support that.
*/
- Multiple<FileParam> files;
+ Multiple<FileParam, AtMost<3> > files;
/**
* On child-process termination, if this LLProcess object still
* exists, post LLSD event to LLEventPump with specified name (default
@@ -217,9 +237,8 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
static LLProcessPtr create(const LLSDOrParams& params);
virtual ~LLProcess();
- // isRunning() isn't const because, when child terminates, it sets stored
- // Status
- bool isRunning(void);
+ /// Is child process still running?
+ bool isRunning() const;
/**
* State of child process
@@ -252,11 +271,11 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
};
/// Status query
- Status getStatus();
+ Status getStatus() const;
/// English Status string query, for logging etc.
- std::string getStatusString();
+ std::string getStatusString() const;
/// English Status string query for previously-captured Status
- std::string getStatusString(const Status& status);
+ std::string getStatusString(const Status& status) const;
/// static English Status string query
static std::string getStatusString(const std::string& desc, const Status& status);
@@ -311,7 +330,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
* filesystem name for the specified pipe. Otherwise returns the empty
* string. @see LLProcess::FileParam::type
*/
- std::string getPipeName(FILESLOT);
+ std::string getPipeName(FILESLOT) const;
/// base of ReadPipe, WritePipe
class LL_COMMON_API BasePipe
@@ -419,7 +438,7 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
* contain "data"="abcde". However, you may still read the entire
* "abcdef" from get_istream(): this limit affects only the size of
* the data posted with the LLSD event. If you don't call this method,
- * all pending data will be posted.
+ * @em no data will be posted: the default is 0 bytes.
*/
virtual void setLimit(size_type limit) = 0;
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index fe599e78921f6ac8381e4d77cedbaf43befcbbd6..d7feddd26b243a6e85cf79ba8ce8a50939f7507b 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -696,7 +696,7 @@ namespace tut
"syntax_error:\n");
py.mParams.files.add(LLProcess::FileParam()); // inherit stdin
py.mParams.files.add(LLProcess::FileParam()); // inherit stdout
- py.mParams.files.add(LLProcess::FileParam("pipe")); // pipe for stderr
+ py.mParams.files.add(LLProcess::FileParam().type("pipe")); // pipe for stderr
py.run();
ensure_equals("Status.mState", py.mPy->getStatus().mState, LLProcess::EXITED);
ensure_equals("Status.mData", py.mPy->getStatus().mData, 1);
@@ -1088,6 +1088,8 @@ namespace tut
py.launch();
std::ostream& childin(py.mPy->getWritePipe(LLProcess::STDIN).get_ostream());
LLProcess::ReadPipe& childout(py.mPy->getReadPipe(LLProcess::STDOUT));
+ // lift the default limit; allow event to carry (some of) the actual data
+ childout.setLimit(20);
// listen for incoming data on childout
EventListener listener(childout.getPump());
// also listen with a function that prompts the child to continue