Preliminary pipe support for LLProcess.

Add LLProcess::FileParam to specify how to construct each child's standard
file slot, with lots of comments about features designed but not yet
implemented. The point is to design it with enough flexibility to be able to
extend to foreseeable use cases.
Add LLProcess::Params::files to collect up to 3 FileParam items. Naturally
this extends the accepted LLSD syntax as well.
Implement type="" (child inherits parent file descriptor) and "pipe" (parent
constructs anonymous pipe to pass to child).
Add LLProcess::FILESLOT enum, plus methods:
getReadPipe(FILESLOT), getOptReadPipe(FILESLOT)
getWritePipe(), getOptWritePipe()
getPipeName(FILESLOT): placeholder implementation for now
Add LLProcess::ReadPipe and WritePipe classes, as returned by get*Pipe().
WritePipe supports get_ostream() method for streaming to child stdin.
ReadPipe supports get_istream() method for reading from child stdout/stderr.
It also provides getPump() returning LLEventPump& so interested parties can
listen for arrival of new data on the aforementioned std::istream.
For "pipe" slots, instantiate appropriate *Pipe class.
ReadPipe and WritePipe classes are pure virtual bases for ReadPipeImpl and
WritePipeImpl, respectively: all implementation data are hidden in the latter
classes, visible only in llprocess.cpp. In fact each *PipeImpl class registers
itself for "mainloop" ticks, attempting nonblocking I/O to the underlying
apr_file_t on each tick. Data are buffered in a boost::asio::streambuf, which
bridges between std::[io]stream and the APR I/O calls.
Sanity-test ReadPipeImpl by using a pipe to absorb the Python "SyntaxError"
output from the successful syntax_error test, rather than alarming the user.
Add first few unit tests for validating FileParam. More tests coming!

3 files changed, 695 insertions, 32 deletions

diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index b13e8eb8e0..55eb7e69d3 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -26,6 +26,7 @@
 
 #include "linden_common.h"
 #include "llprocess.h"
+#include "llsdutil.h"
 #include "llsdserialize.h"
 #include "llsingleton.h"
 #include "llstring.h"
@@ -36,19 +37,20 @@
 
 #include <boost/foreach.hpp>
 #include <boost/bind.hpp>
+#include <boost/asio/streambuf.hpp>
+#include <boost/asio/buffers_iterator.hpp>
 #include <iostream>
 #include <stdexcept>
+#include <limits>
+#include <algorithm>
+#include <vector>
+#include <typeinfo>
+#include <utility>
 
+static const char* whichfile[] = { "stdin", "stdout", "stderr" };
 static std::string empty;
 static LLProcess::Status interpret_status(int status);
 
-/// Need an exception to avoid constructing an invalid LLProcess object, but
-/// internal use only
-struct LLProcessError: public std::runtime_error
-{
-	LLProcessError(const std::string& msg): std::runtime_error(msg) {}
-};
-
 /**
  * Ref-counted "mainloop" listener. As long as there are still outstanding
  * LLProcess objects, keep listening on "mainloop" so we can keep polling APR
@@ -117,6 +119,154 @@ private:
 };
 static LLProcessListener sProcessListener;
 
+LLProcess::BasePipe::~BasePipe() {}
+
+class WritePipeImpl: public LLProcess::WritePipe
+{
+public:
+	WritePipeImpl(const std::string& desc, apr_file_t* pipe):
+		mDesc(desc),
+		mPipe(pipe),
+		// Essential to initialize our std::ostream with our special streambuf!
+		mStream(&mStreambuf)
+	{
+		mConnection = LLEventPumps::instance().obtain("mainloop")
+			.listen(LLEventPump::inventName("WritePipe"),
+					boost::bind(&WritePipeImpl::tick, this, _1));
+	}
+
+	virtual std::ostream& get_ostream() { return mStream; }
+
+	bool tick(const LLSD&)
+	{
+		// If there's anything to send, try to send it.
+		if (mStreambuf.size())
+		{
+			// Copy data out from mStreambuf to a flat, contiguous buffer to
+			// write -- but only up to a certain size.
+			std::streamsize total(mStreambuf.size());
+			std::streamsize bufsize((std::min)(4096, total));
+			boost::asio::streambuf::const_buffers_type bufs = mStreambuf.data();
+			std::vector<char> buffer(
+				boost::asio::buffers_begin(bufs),
+				boost::asio::buffers_begin(bufs) + bufsize);
+			apr_size_t written(bufsize);
+			ll_apr_warn_status(apr_file_write(mPipe, &buffer[0], &written));
+			// 'written' is modified to reflect the number of bytes actually
+			// written. Since they've been sent, remove them from the
+			// streambuf so we don't keep trying to send them. This could be
+			// anywhere from 0 up to mStreambuf.size(); anything we haven't
+			// yet sent, we'll try again next tick() call.
+			mStreambuf.consume(written);
+			LL_DEBUGS("LLProcess") << "wrote " << written << " of " << bufsize
+								   << " bytes to " << mDesc
+								   << " (original " << total << "), "
+								   << mStreambuf.size() << " remaining" << LL_ENDL;
+		}
+		return false;
+	}
+
+private:
+	std::string mDesc;
+	apr_file_t* mPipe;
+	LLTempBoundListener mConnection;
+	boost::asio::streambuf mStreambuf;
+	std::ostream mStream;
+};
+
+class ReadPipeImpl: public LLProcess::ReadPipe
+{
+public:
+	ReadPipeImpl(const std::string& desc, apr_file_t* pipe):
+		mDesc(desc),
+		mPipe(pipe),
+		// Essential to initialize our std::istream with our special streambuf!
+		mStream(&mStreambuf),
+		mPump("ReadPipe"),
+		// use funky syntax to call max() to avoid blighted max() macros
+		mLimit((std::numeric_limits<size_t>::max)())
+	{
+		mConnection = LLEventPumps::instance().obtain("mainloop")
+			.listen(LLEventPump::inventName("ReadPipe"),
+					boost::bind(&ReadPipeImpl::tick, this, _1));
+	}
+
+	// Much of the implementation is simply connecting the abstract virtual
+	// methods with implementation data concealed from the base class.
+	virtual std::istream& get_istream() { return mStream; }
+	virtual LLEventPump& getPump() { return mPump; }
+	virtual void setLimit(size_t limit) { mLimit = limit; }
+	virtual size_t getLimit() const { return mLimit; }
+
+private:
+	bool tick(const LLSD&)
+	{
+		// Allocate a buffer and try, every time, to read into it.
+		std::vector<char> buffer(4096);
+		apr_size_t gotten(buffer.size());
+		apr_status_t err = apr_file_read(mPipe, &buffer[0], &gotten);
+		if (err == APR_EOF)
+		{
+			// Handle EOF specially: it's part of normal-case processing.
+			LL_DEBUGS("LLProcess") << "EOF on " << mDesc << LL_ENDL;
+			// We won't need any more tick() calls.
+			mConnection.disconnect();
+		}
+		else if (! ll_apr_warn_status(err)) // validate anything but EOF
+		{
+			// 'gotten' was modified to reflect the number of bytes actually
+			// received. If nonzero, add them to the streambuf and notify
+			// interested parties.
+			if (gotten)
+			{
+				boost::asio::streambuf::mutable_buffers_type mbufs = mStreambuf.prepare(gotten);
+				std::copy(buffer.begin(), buffer.begin() + gotten,
+						  boost::asio::buffers_begin(mbufs));
+				// Don't forget to "commit" the data! The sequence (prepare(),
+				// commit()) is obviously intended to allow us to allocate
+				// buffer space, then read directly into some portion of it,
+				// then commit only as much as we managed to obtain. But the
+				// only official (documented) way I can find to populate a
+				// mutable_buffers_type is to use buffers_begin(). It Would Be
+				// Nice if we were permitted to directly read into
+				// mutable_buffers_type (not to mention writing directly from
+				// const_buffers_type in WritePipeImpl; APR even supports an
+				// apr_file_writev() function for writing from discontiguous
+				// buffers) -- but as of 2012-02-14, this copying appears to
+				// be the safest tactic.
+				mStreambuf.commit(gotten);
+				LL_DEBUGS("LLProcess") << "read " << gotten << " of " << buffer.size()
+									   << " bytes from " << mDesc << ", new total "
+									   << mStreambuf.size() << LL_ENDL;
+
+				// Now that we've received new data, publish it on our
+				// LLEventPump as advertised. Constrain it by mLimit.
+				std::streamsize datasize((std::min)(mLimit, mStreambuf.size()));
+				boost::asio::streambuf::const_buffers_type cbufs = mStreambuf.data();
+				mPump.post(LLSDMap("data", LLSD::String(
+									   boost::asio::buffers_begin(cbufs),
+									   boost::asio::buffers_begin(cbufs) + datasize)));
+			}
+		}
+		return false;
+	}
+
+	std::string mDesc;
+	apr_file_t* mPipe;
+	LLTempBoundListener mConnection;
+	boost::asio::streambuf mStreambuf;
+	std::istream mStream;
+	LLEventStream mPump;
+	size_t mLimit;
+};
+
+/// Need an exception to avoid constructing an invalid LLProcess object, but
+/// internal use only
+struct LLProcessError: public std::runtime_error
+{
+	LLProcessError(const std::string& msg): std::runtime_error(msg) {}
+};
+
 LLProcessPtr LLProcess::create(const LLSDOrParams& params)
 {
 	try
@@ -134,12 +284,23 @@ LLProcessPtr LLProcess::create(const LLSDOrParams& params)
 /// throw LLProcessError mentioning the function call that produced that
 /// result.
 #define chkapr(func)                            \
-    if (ll_apr_warn_status(func))               \
-        throw LLProcessError(#func " failed")
+	if (ll_apr_warn_status(func))				\
+		throw LLProcessError(#func " failed")
 
 LLProcess::LLProcess(const LLSDOrParams& params):
-	mAutokill(params.autokill)
+	mAutokill(params.autokill),
+	mPipes(NSLOTS)
 {
+	// Hmm, when you construct a ptr_vector with a size, it merely reserves
+	// space, it doesn't actually make it that big. Explicitly make it bigger.
+	// Because of ptr_vector's odd semantics, have to push_back(0) the right
+	// number of times! resize() wants to default-construct new BasePipe
+	// instances, which fails because it's pure virtual. But because of the
+	// constructor call, these push_back() calls should require no new
+	// allocation.
+	for (size_t i = 0; i < mPipes.capacity(); ++i)
+		mPipes.push_back(0);
+
 	if (! params.validateBlock(true))
 	{
 		throw LLProcessError(STRINGIZE("not launched: failed parameter validation\n"
@@ -154,16 +315,46 @@ LLProcess::LLProcess(const LLSDOrParams& params):
 	// apr_procattr_io_set() alternatives: inherit the viewer's own stdxxx
 	// handle (APR_NO_PIPE, e.g. for stdout, stderr), or create a pipe that's
 	// blocking on the child end but nonblocking at the viewer end
-	// (APR_CHILD_BLOCK). The viewer can't block for anything: the parent end
-	// MUST be nonblocking. As the APR documentation itself points out, it
-	// 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.
+	// (APR_CHILD_BLOCK).
 	// Other major options could include explicitly creating a single APR pipe
 	// 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).
-//	chkapr(apr_procattr_io_set(procattr, APR_CHILD_BLOCK, APR_CHILD_BLOCK, APR_CHILD_BLOCK));
-	chkapr(apr_procattr_io_set(procattr, APR_NO_PIPE, APR_NO_PIPE, APR_NO_PIPE));
+	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)
+	{
+		if (std::string(fparams[i].type).empty()) // inherit our file descriptor
+		{
+			select[i] = APR_NO_PIPE;
+		}
+		else if (std::string(fparams[i].type) == "pipe") // anonymous pipe
+		{
+			if (! std::string(fparams[i].name).empty())
+			{
+				LL_WARNS("LLProcess") << "For " << std::string(params.executable)
+									  << ": internal names for reusing pipes ('"
+									  << std::string(fparams[i].name) << "' for " << whichfile[i]
+									  << ") are not yet supported -- creating distinct pipe"
+									  << LL_ENDL;
+			}
+			// The viewer can't block for anything: the parent end MUST be
+			// nonblocking. As the APR documentation itself points out, it
+			// 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;
+		}
+		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) << "'"));
+		}
+	}
+	chkapr(apr_procattr_io_set(procattr, select[STDIN], select[STDOUT], select[STDERR]));
 
 	// Thumbs down on implicitly invoking the shell to invoke the child. From
 	// our point of view, the other major alternative to APR_PROGRAM_PATH
@@ -251,6 +442,27 @@ LLProcess::LLProcess(const LLSDOrParams& params):
 		// On Windows, associate the new child process with our Job Object.
 		autokill();
 	}
+
+	// Instantiate the proper pipe I/O machinery
+	// want to be able to point to apr_proc_t::in, out, err by index
+	typedef apr_file_t* apr_proc_t::*apr_proc_file_ptr;
+	static apr_proc_file_ptr members[] =
+		{ &apr_proc_t::in, &apr_proc_t::out, &apr_proc_t::err };
+	for (size_t i = 0; i < NSLOTS; ++i)
+	{
+		if (select[i] != APR_CHILD_BLOCK)
+			continue;
+		if (i == STDIN)
+		{
+			mPipes.replace(i, new WritePipeImpl(whichfile[i], mProcess.*(members[i])));
+		}
+		else
+		{
+			mPipes.replace(i, new ReadPipeImpl(whichfile[i], mProcess.*(members[i])));
+		}
+		LL_DEBUGS("LLProcess") << "Instantiating " << typeid(mPipes[i]).name()
+							   << "('" << whichfile[i] << "')" << LL_ENDL;
+	}
 }
 
 LLProcess::~LLProcess()
@@ -428,6 +640,83 @@ LLProcess::handle LLProcess::getProcessHandle() const
 #endif
 }
 
+std::string LLProcess::getPipeName(FILESLOT)
+{
+	// LLProcess::FileParam::type "npipe" is not yet implemented
+	return "";
+}
+
+template<class PIPETYPE>
+PIPETYPE* LLProcess::getPipePtr(std::string& error, FILESLOT slot)
+{
+    if (slot >= NSLOTS)
+    {
+        error = STRINGIZE(mDesc << " has no slot " << slot);
+        return NULL;
+    }
+    if (mPipes.is_null(slot))
+    {
+        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
+    // accepting runtime's exception.
+    PIPETYPE* ppipe = dynamic_cast<PIPETYPE*>(&mPipes[slot]);
+    if (! ppipe)
+    {
+        error = STRINGIZE(mDesc << ' ' << whichfile[slot] << " not a " << typeid(PIPETYPE).name());
+        return NULL;
+    }
+
+    error.clear();
+    return ppipe;
+}
+
+template <class PIPETYPE>
+PIPETYPE& LLProcess::getPipe(FILESLOT slot)
+{
+    std::string error;
+    PIPETYPE* wp = getPipePtr<PIPETYPE>(error, slot);
+    if (! wp)
+    {
+        throw NoPipe(error);
+    }
+    return *wp;
+}
+
+template <class PIPETYPE>
+boost::optional<PIPETYPE&> LLProcess::getOptPipe(FILESLOT slot)
+{
+    std::string error;
+    PIPETYPE* wp = getPipePtr<PIPETYPE>(error, slot);
+    if (! wp)
+    {
+        LL_DEBUGS("LLProcess") << error << LL_ENDL;
+        return boost::optional<PIPETYPE&>();
+    }
+    return *wp;
+}
+
+LLProcess::WritePipe& LLProcess::getWritePipe(FILESLOT slot)
+{
+    return getPipe<WritePipe>(slot);
+}
+
+boost::optional<LLProcess::WritePipe&> LLProcess::getOptWritePipe(FILESLOT slot)
+{
+    return getOptPipe<WritePipe>(slot);
+}
+
+LLProcess::ReadPipe& LLProcess::getReadPipe(FILESLOT slot)
+{
+    return getPipe<ReadPipe>(slot);
+}
+
+boost::optional<LLProcess::ReadPipe&> LLProcess::getOptReadPipe(FILESLOT slot)
+{
+    return getOptPipe<ReadPipe>(slot);
+}
+
 std::ostream& operator<<(std::ostream& out, const LLProcess::Params& params)
 {
 	std::string cwd(params.cwd);
diff --git a/indra/llcommon/llprocess.h b/indra/llcommon/llprocess.h
index b95ae55701..448a88f4c0 100644
--- a/indra/llcommon/llprocess.h
+++ b/indra/llcommon/llprocess.h
@@ -31,8 +31,11 @@
 #include "llsdparam.h"
 #include "apr_thread_proc.h"
 #include <boost/shared_ptr.hpp>
+#include <boost/ptr_container/ptr_vector.hpp>
+#include <boost/optional.hpp>
 #include <boost/noncopyable.hpp>
 #include <iosfwd>                   // std::ostream
+#include <stdexcept>
 
 #if LL_WINDOWS
 #define WIN32_LEAN_AND_MEAN
@@ -43,6 +46,8 @@
 #endif
 #endif
 
+class LLEventPump;
+
 class LLProcess;
 /// LLProcess instances are created on the heap by static factory methods and
 /// managed by ref-counted pointers.
@@ -64,6 +69,87 @@ class LL_COMMON_API LLProcess: public boost::noncopyable
 {
 	LOG_CLASS(LLProcess);
 public:
+	/**
+	 * Specify what to pass for each of child stdin, stdout, stderr.
+	 * @see LLProcess::Params::files.
+	 */
+	struct FileParam: public LLInitParam::Block<FileParam>
+	{
+		/**
+		 * type of file handle to pass to child process
+		 *
+		 * - "" (default): let the child inherit the same file handle used by
+		 *   this process. For instance, if passed as stdout, child stdout
+		 *   will be interleaved with stdout from this process. In this case,
+		 *   @a name is moot and should be left "".
+		 *
+		 * - "file": open an OS filesystem file with the specified @a name.
+		 *   <i>Not yet implemented.</i>
+		 *
+		 * - "pipe" or "tpipe" or "npipe": depends on @a name
+		 *
+		 *   - @a name.empty(): construct an OS pipe used only for this slot
+		 *     of the forthcoming child process.
+		 *
+		 *   - ! @a name.empty(): in a global registry, find or create (using
+		 *     the specified @a name) an OS pipe. The point of the (purely
+		 *     internal) @a name is that passing the same @a name in more than
+		 *     one slot for a given LLProcess -- or for slots in different
+		 *     LLProcess instances -- means the same pipe. For example, you
+		 *     might pass the same @a name value as both stdout and stderr to
+		 *     make the child process produce both on the same actual pipe. Or
+		 *     you might pass the same @a name as the stdout for one LLProcess
+		 *     and the stdin for another to connect the two child processes.
+		 *     Use LLProcess::getPipeName() to generate a unique name
+		 *     guaranteed not to already exist in the registry. <i>Not yet
+		 *     implemented.</i>
+		 *
+		 *   The difference between "pipe", "tpipe" and "npipe" is as follows.
+		 *
+		 *   - "pipe": direct LLProcess to monitor the parent end of the pipe,
+		 *     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.
+		 *
+		 *   - "tpipe": do not engage LLProcess machinery to monitor the
+		 *     parent end of the pipe. A "tpipe" is used only to connect
+		 *     different child processes. As such, it makes little sense to
+		 *     pass an empty @a name. <i>Not yet implemented.</i>
+		 *
+		 *   - "npipe": like "tpipe", but use an OS named pipe with a
+		 *     generated name. Note that @a name is the @em internal name of
+		 *     the pipe in our global registry -- it doesn't necessarily have
+		 *     anything to do with the pipe's name in the OS filesystem. Use
+		 *     LLProcess::getPipeName() to obtain the named pipe's OS
+		 *     filesystem name, e.g. to pass it as the @a name to another
+		 *     LLProcess instance using @a type "file". This supports usage
+		 *     like bash's &lt;(subcommand...) or &gt;(subcommand...)
+		 *     constructs. <i>Not yet implemented.</i>
+		 *
+		 * In all cases the open mode (read, write) is determined by the child
+		 * slot you're filling. Child stdin means select the "read" end of a
+		 * pipe, or open a filesystem file for reading; child stdout or stderr
+		 * means select the "write" end of a pipe, or open a filesystem file
+		 * for writing.
+		 *
+		 * Confusion such as passing the same pipe as the stdin of two
+		 * processes (rather than stdout for one and stdin for the other) is
+		 * explicitly permitted: it's up to the caller to construct meaningful
+		 * LLProcess pipe graphs.
+		 */
+		Optional<std::string> type;
+		Optional<std::string> name;
+
+		FileParam(const std::string& tp="", const std::string& nm=""):
+			type("type", tp),
+			name("name", nm)
+		{}
+	};
+
 	/// Param block definition
 	struct Params: public LLInitParam::Block<Params>
 	{
@@ -71,7 +157,8 @@ public:
 			executable("executable"),
 			args("args"),
 			cwd("cwd"),
-			autokill("autokill", true)
+			autokill("autokill", true),
+			files("files")
 		{}
 
 		/// pathname of executable
@@ -87,19 +174,22 @@ public:
 		Optional<std::string> cwd;
 		/// implicitly kill process on destruction of LLProcess object
 		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 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.
+		 */
+		Multiple<FileParam> files;
 	};
 	typedef LLSDParamAdapter<Params> LLSDOrParams;
 
 	/**
 	 * Factory accepting either plain LLSD::Map or Params block.
 	 * MAY RETURN DEFAULT-CONSTRUCTED LLProcessPtr if params invalid!
-	 *
-	 * Redundant with Params definition above?
-	 *
-	 * executable (required, string):				executable pathname
-	 * args		  (optional, string array):			extra command-line arguments
-	 * cwd		  (optional, string, dft no chdir): change to this directory before executing
-	 * autokill	  (optional, bool, dft true):		implicit kill() on ~LLProcess
 	 */
 	static LLProcessPtr create(const LLSDOrParams& params);
 	virtual ~LLProcess();
@@ -190,6 +280,125 @@ public:
 	 */
 	static handle isRunning(handle, const std::string& desc="");
 
+	/// Provide symbolic access to child's file slots
+	enum FILESLOT { STDIN=0, STDOUT=1, STDERR=2, NSLOTS=3 };
+
+	/**
+	 * For a pipe constructed with @a type "npipe", obtain the generated OS
+	 * filesystem name for the specified pipe. Otherwise returns the empty
+	 * string. @see LLProcess::FileParam::type
+	 */
+	std::string getPipeName(FILESLOT);
+
+	/// base of ReadPipe, WritePipe
+	class BasePipe
+	{
+	public:
+		virtual ~BasePipe() = 0;
+	};
+
+	/// As returned by getWritePipe() or getOptWritePipe()
+	class WritePipe: public BasePipe
+	{
+	public:
+		/**
+		 * Get ostream& on which to write to child's stdin.
+		 *
+		 * @usage
+		 * @code
+		 * myProcess->getWritePipe().get_ostream() << "Hello, child!" << std::endl;
+		 * @endcode
+		 */
+		virtual std::ostream& get_ostream() = 0;
+	};
+
+	/// As returned by getReadPipe() or getOptReadPipe()
+	class ReadPipe: public BasePipe
+	{
+	public:
+		/**
+		 * Get istream& on which to read from child's stdout or stderr.
+		 *
+		 * @usage
+		 * @code
+		 * std::string stuff;
+		 * myProcess->getReadPipe().get_istream() >> stuff;
+		 * @endcode
+		 *
+		 * You should be sure in advance that the ReadPipe in question can
+		 * fill the request. @see getPump()
+		 */
+		virtual std::istream& get_istream() = 0;
+
+		/**
+		 * Get LLEventPump& on which to listen for incoming data. The posted
+		 * LLSD::Map event will contain a key "data" whose value is an
+		 * LLSD::String containing (part of) the data accumulated in the
+		 * buffer.
+		 *
+		 * If the child sends "abc", and this ReadPipe posts "data"="abc", but
+		 * you don't consume it by reading the std::istream returned by
+		 * get_istream(), and the child next sends "def", ReadPipe will post
+		 * "data"="abcdef".
+		 */
+		virtual LLEventPump& getPump() = 0;
+
+		/**
+		 * Set maximum length of buffer data that will be posted in the LLSD
+		 * announcing arrival of new data from the child. If you call
+		 * setLimit(5), and the child sends "abcdef", the LLSD event will
+		 * 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.
+		 */
+		virtual void setLimit(size_t limit) = 0;
+
+		/**
+		 * Query the current setLimit() limit.
+		 */
+		virtual size_t getLimit() const = 0;
+	};
+
+	/// Exception thrown by getWritePipe(), getReadPipe() if you didn't ask to
+	/// create a pipe at the corresponding FILESLOT.
+	struct NoPipe: public std::runtime_error
+	{
+		NoPipe(const std::string& what): std::runtime_error(what) {}
+	};
+
+	/**
+	 * Get a reference to the (only) WritePipe for this LLProcess. @a slot, if
+	 * specified, must be STDIN. Throws NoPipe if you did not request a "pipe"
+	 * for child stdin. Use this method when you know how you created the
+	 * LLProcess in hand.
+	 */
+	WritePipe& getWritePipe(FILESLOT slot=STDIN);
+
+	/**
+	 * Get a boost::optional<WritePipe&> to the (only) WritePipe for this
+	 * LLProcess. @a slot, if specified, must be STDIN. The return value is
+	 * empty if you did not request a "pipe" for child stdin. Use this method
+	 * for inspecting an LLProcess you did not create.
+	 */
+	boost::optional<WritePipe&> getOptWritePipe(FILESLOT slot=STDIN);
+
+	/**
+	 * Get a reference to one of the ReadPipes for this LLProcess. @a slot, if
+	 * specified, must be STDOUT or STDERR. Throws NoPipe if you did not
+	 * request a "pipe" for child stdout or stderr. Use this method when you
+	 * know how you created the LLProcess in hand.
+	 */
+	ReadPipe& getReadPipe(FILESLOT slot);
+
+	/**
+	 * Get a boost::optional<ReadPipe&> to one of the ReadPipes for this
+	 * LLProcess. @a slot, if specified, must be STDOUT or STDERR. The return
+	 * value is empty if you did not request a "pipe" for child stdout or
+	 * stderr. Use this method for inspecting an LLProcess you did not create.
+	 */
+	boost::optional<ReadPipe&> getOptReadPipe(FILESLOT slot);
+
 private:
 	/// constructor is private: use create() instead
 	LLProcess(const LLSDOrParams& params);
@@ -198,11 +407,21 @@ private:
 	static void status_callback(int reason, void* data, int status);
 	// Object-oriented callback
 	void handle_status(int reason, int status);
+	// implementation for get[Opt][Read|Write]Pipe()
+	template <class PIPETYPE>
+	PIPETYPE& getPipe(FILESLOT slot);
+	template <class PIPETYPE>
+	boost::optional<PIPETYPE&> getOptPipe(FILESLOT slot);
+	template <class PIPETYPE>
+	PIPETYPE* getPipePtr(std::string& error, FILESLOT slot);
 
 	std::string mDesc;
 	apr_proc_t mProcess;
 	bool mAutokill;
 	Status mStatus;
+	// explicitly want this ptr_vector to be able to store NULLs
+	typedef boost::ptr_vector< boost::nullable<BasePipe> > PipeVector;
+	PipeVector mPipes;
 };
 
 /// for logging
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 8c21be196b..d4e9977e63 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -34,6 +34,7 @@
 #include "stringize.h"
 #include "llsdutil.h"
 #include "llevents.h"
+#include "llerrorcontrol.h"
 
 #if defined(LL_WINDOWS)
 #define sleep(secs) _sleep((secs) * 1000)
@@ -92,12 +93,18 @@ static std::string readfile(const std::string& pathname, const std::string& desc
 /// Looping on LLProcess::isRunning() must now be accompanied by pumping
 /// "mainloop" -- otherwise the status won't update and you get an infinite
 /// loop.
+void yield(int seconds=1)
+{
+    // This function simulates waiting for another viewer frame
+    sleep(seconds);
+    LLEventPumps::instance().obtain("mainloop").post(LLSD());
+}
+
 void waitfor(LLProcess& proc)
 {
     while (proc.isRunning())
     {
-        sleep(1);
-        LLEventPumps::instance().obtain("mainloop").post(LLSD());
+        yield();
     }
 }
 
@@ -105,8 +112,7 @@ void waitfor(LLProcess::handle h, const std::string& desc)
 {
     while (LLProcess::isRunning(h, desc))
     {
-        sleep(1);
-        LLEventPumps::instance().obtain("mainloop").post(LLSD());
+        yield();
     }
 }
 
@@ -219,6 +225,68 @@ private:
     std::string mPath;
 };
 
+// statically reference the function in test.cpp... it's short, we could
+// replicate, but better to reuse
+extern void wouldHaveCrashed(const std::string& message);
+
+/**
+ * Capture log messages. This is adapted (simplified) from the one in
+ * llerror_test.cpp. Sigh, should've broken that out into a separate header
+ * file, but time for this project is short...
+ */
+class TestRecorder : public LLError::Recorder
+{
+public:
+    TestRecorder():
+        // Mostly what we're trying to accomplish by saving and resetting
+        // LLError::Settings is to bypass the default RecordToStderr and
+        // RecordToWinDebug Recorders. As these are visible only inside
+        // llerror.cpp, we can't just call LLError::removeRecorder() with
+        // each. For certain tests we need to produce, capture and examine
+        // DEBUG log messages -- but we don't want to spam the user's console
+        // with that output. If it turns out that saveAndResetSettings() has
+        // some bad effect, give up and just let the DEBUG level log messages
+        // display.
+        mOldSettings(LLError::saveAndResetSettings())
+    {
+        LLError::setFatalFunction(wouldHaveCrashed);
+        LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
+        LLError::addRecorder(this);
+    }
+
+    ~TestRecorder()
+    {
+        LLError::removeRecorder(this);
+        LLError::restoreSettings(mOldSettings);
+    }
+
+    void recordMessage(LLError::ELevel level,
+                       const std::string& message)
+    {
+        mMessages.push_back(message);
+    }
+
+    /// Don't assume the message we want is necessarily the LAST log message
+    /// emitted by the underlying code; search backwards through all messages
+    /// for the sought string.
+    std::string messageWith(const std::string& search)
+    {
+        for (std::list<std::string>::const_reverse_iterator rmi(mMessages.rbegin()),
+                 rmend(mMessages.rend());
+             rmi != rmend; ++rmi)
+        {
+            if (rmi->find(search) != std::string::npos)
+                return *rmi;
+        }
+        // failed to find any such message
+        return std::string();
+    }
+
+    typedef std::list<std::string> MessageList;
+    MessageList mMessages;
+    LLError::Settings* mOldSettings;
+};
+
 /*****************************************************************************
 *   TUT
 *****************************************************************************/
@@ -602,9 +670,19 @@ namespace tut
         set_test_name("syntax_error:");
         PythonProcessLauncher py("syntax_error:",
                                  "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.run();
         ensure_equals("Status.mState", py.mPy->getStatus().mState, LLProcess::EXITED);
         ensure_equals("Status.mData",  py.mPy->getStatus().mData,  1);
+        std::istream& rpipe(py.mPy->getReadPipe(LLProcess::STDERR).get_istream());
+        std::vector<char> buffer(4096);
+        rpipe.read(&buffer[0], buffer.size());
+        std::streamsize got(rpipe.gcount());
+        ensure("Nothing read from stderr pipe", got);
+        std::string data(&buffer[0], got);
+        ensure("Didn't find 'SyntaxError:'", data.find("\nSyntaxError:") != std::string::npos);
     }
 
     template<> template<>
@@ -629,7 +707,7 @@ namespace tut
         int i = 0, timeout = 60;
         for ( ; i < timeout; ++i)
         {
-            sleep(1);
+            yield();
             if (readfile(out.getName(), "from kill() script") == "ok")
                 break;
         }
@@ -678,7 +756,7 @@ namespace tut
             int i = 0, timeout = 60;
             for ( ; i < timeout; ++i)
             {
-                sleep(1);
+                yield();
                 if (readfile(out.getName(), "from kill() script") == "ok")
                     break;
             }
@@ -733,7 +811,7 @@ namespace tut
             int i = 0, timeout = 60;
             for ( ; i < timeout; ++i)
             {
-                sleep(1);
+                yield();
                 if (readfile(from.getName(), "from autokill script") == "ok")
                     break;
             }
@@ -742,7 +820,7 @@ namespace tut
             // Now destroy the LLProcess, which should NOT kill the child!
         }
         // If the destructor killed the child anyway, give it time to die
-        sleep(2);
+        yield(2);
         // How do we know it's not terminated? By making it respond to
         // a specific stimulus in a specific way.
         {
@@ -755,4 +833,81 @@ namespace tut
         // script could not have written 'ack' as we expect.
         ensure_equals("autokill script output", readfile(from.getName()), "ack");
     }
+
+    template<> template<>
+    void object::test<10>()
+    {
+        set_test_name("'bogus' test");
+        TestRecorder recorder;
+        PythonProcessLauncher py("'bogus' test",
+                                 "print 'Hello world'\n");
+        py.mParams.files.add(LLProcess::FileParam("bogus"));
+        py.mPy = LLProcess::create(py.mParams);
+        ensure("should have rejected 'bogus'", ! py.mPy);
+        std::string message(recorder.messageWith("bogus"));
+        ensure("did not log 'bogus' type", ! message.empty());
+        ensure_contains("did not name 'stdin'", message, "stdin");
+    }
+
+    template<> template<>
+    void object::test<11>()
+    {
+        set_test_name("'file' test");
+        // Replace this test with one or more real 'file' tests when we
+        // implement 'file' support
+        PythonProcessLauncher py("'file' test",
+                                 "print 'Hello world'\n");
+        py.mParams.files.add(LLProcess::FileParam());
+        py.mParams.files.add(LLProcess::FileParam("file"));
+        py.mPy = LLProcess::create(py.mParams);
+        ensure("should have rejected 'file'", ! py.mPy);
+    }
+
+    template<> template<>
+    void object::test<12>()
+    {
+        set_test_name("'tpipe' test");
+        // Replace this test with one or more real 'tpipe' tests when we
+        // implement 'tpipe' support
+        TestRecorder recorder;
+        PythonProcessLauncher py("'tpipe' test",
+                                 "print 'Hello world'\n");
+        py.mParams.files.add(LLProcess::FileParam());
+        py.mParams.files.add(LLProcess::FileParam("tpipe"));
+        py.mPy = LLProcess::create(py.mParams);
+        ensure("should have rejected 'tpipe'", ! py.mPy);
+        std::string message(recorder.messageWith("tpipe"));
+        ensure("did not log 'tpipe' type", ! message.empty());
+        ensure_contains("did not name 'stdout'", message, "stdout");
+    }
+
+    template<> template<>
+    void object::test<13>()
+    {
+        set_test_name("'npipe' test");
+        // Replace this test with one or more real 'npipe' tests when we
+        // implement 'npipe' support
+        TestRecorder recorder;
+        PythonProcessLauncher py("'npipe' test",
+                                 "print 'Hello world'\n");
+        py.mParams.files.add(LLProcess::FileParam());
+        py.mParams.files.add(LLProcess::FileParam());
+        py.mParams.files.add(LLProcess::FileParam("npipe"));
+        py.mPy = LLProcess::create(py.mParams);
+        ensure("should have rejected 'npipe'", ! py.mPy);
+        std::string message(recorder.messageWith("npipe"));
+        ensure("did not log 'npipe' type", ! message.empty());
+        ensure_contains("did not name 'stderr'", message, "stderr");
+    }
+
+    // TODO:
+    // test "pipe" with nonempty name (should log & continue)
+    // test pipe for just stderr (tests for get[Opt]ReadPipe(), get[Opt]WritePipe())
+    // test pipe for stdin, stdout (etc.)
+    // test getWritePipe().get_ostream(), getReadPipe().get_istream()
+    // test listening on getReadPipe().getPump(), disconnecting
+    // test setLimit(), getLimit()
+    // test EOF -- check logging
+    // test get(Read|Write)Pipe(3), unmonitored slot, getReadPipe(1), getWritePipe(0)
+
 } // namespace tut