Skip to content
Snippets Groups Projects
Unverified Commit d317454c authored by nat-goodspeed's avatar nat-goodspeed Committed by GitHub
Browse files

Merge pull request #1639 from secondlife/nat/stack-overflow 

Make LLLeap iterate over child stdout while read data is pending
parents 83e80d79 391893a3
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
indra/llcommon/llleap.cpp
+ 108
− 112
View file @ d317454c
......@@ -18,9 +18,6 @@
#include <algorithm>
// std headers
// external library headers
#include <boost/bind.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/tokenizer.hpp>
// other Linden headers
#include "llerror.h"
#include "llstring.h"
......@@ -64,7 +61,9 @@ class LLLeapImpl: public LLLeap
// Pass it a callback to our connect() method, so it can send events
// from a particular LLEventPump to the plugin without having to know
// this class or method name.
mListener(new LLLeapListener(boost::bind(&LLLeapImpl::connect, this, _1, _2)))
mListener(new LLLeapListener(
[this](LLEventPump& pump, const std::string& listener)
{ return connect(pump, listener); }))
{
// Rule out unpopulated Params block
if (! cparams.executable.isProvided())
......@@ -93,7 +92,7 @@ class LLLeapImpl: public LLLeap
}
// Listen for child "termination" right away to catch launch errors.
mDonePump.listen("LLLeap", boost::bind(&LLLeapImpl::bad_launch, this, _1));
mDonePump.listen("LLLeap", [this](const LLSD& data){ return bad_launch(data); });
// Okay, launch child.
// Get a modifiable copy of params block to set files and postend.
......@@ -113,7 +112,7 @@ class LLLeapImpl: public LLLeap
// Okay, launch apparently worked. Change our mDonePump listener.
mDonePump.stopListening("LLLeap");
mDonePump.listen("LLLeap", boost::bind(&LLLeapImpl::done, this, _1));
mDonePump.listen("LLLeap", [this](const LLSD& data){ return done(data); });
// Child might pump large volumes of data through either stdout or
// stderr. Don't bother copying all that data into notification event.
......@@ -128,13 +127,9 @@ class LLLeapImpl: public LLLeap
// Listening on stdout is stateful. In general, we're either waiting
// for the length prefix or waiting for the specified length of data.
// We address that with two different listener methods -- one of which
// is blocked at any given time.
mReadPrefix = true;
mStdoutConnection = childout.getPump()
.listen("prefix", boost::bind(&LLLeapImpl::rstdout, this, _1));
mStdoutDataConnection = childout.getPump()
.listen("data", boost::bind(&LLLeapImpl::rstdoutData, this, _1));
mBlocker.reset(new LLEventPump::Blocker(mStdoutDataConnection));
.listen("LLLeap", [this](const LLSD& data){ return rstdout(data); });
// Log anything sent up through stderr. When a typical program
// encounters an error, it writes its error message to stderr and
......@@ -142,7 +137,7 @@ class LLLeapImpl: public LLLeap
// interpreter behaves that way. More generally, though, a plugin
// author can log whatever s/he wants to the viewer log using stderr.
mStderrConnection = childerr.getPump()
.listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1));
.listen("LLLeap", [this](const LLSD& data){ return rstderr(data); });
// For our lifespan, intercept any LL_ERRS so we can notify plugin
mRecorder = LLError::addGenericRecorder(
......@@ -255,120 +250,120 @@ class LLLeapImpl: public LLLeap
return false;
}
// Initial state of stateful listening on child stdout: wait for a length
// prefix, followed by ':'.
bool rstdout(const LLSD& data)
// Stateful listening on child stdout:
// wait for a length prefix, followed by ':'.
bool rstdout(const LLSD&)
{
LLProcess::ReadPipe& childout(mChild->getReadPipe(LLProcess::STDOUT));
// It's possible we got notified of a couple digit characters without
// seeing the ':' -- unlikely, but still. Until we see ':', keep
// waiting.
if (childout.contains(':'))
while (childout.size())
{
std::istream& childstream(childout.get_istream());
// Saw ':', read length prefix and store in mExpect.
size_t expect;
childstream >> expect;
int colon(childstream.get());
if (colon != ':')
/*----------------- waiting for length prefix ------------------*/
if (mReadPrefix)
{
// Protocol failure. Clear out the rest of the pending data in
// childout (well, up to a max length) to log what was wrong.
LLProcess::ReadPipe::size_type
readlen((std::min)(childout.size(), LLProcess::ReadPipe::size_type(80)));
bad_protocol(STRINGIZE(expect << char(colon) << childout.read(readlen)));
// It's possible we got notified of a couple digit characters without
// seeing the ':' -- unlikely, but still. Until we see ':', keep
// waiting.
if (! childout.contains(':'))
{
if (childout.contains('\n'))
{
// Since this is the initial listening state, this is where we'd
// arrive if the child isn't following protocol at all -- say
// because the user specified 'ls' or some darn thing.
bad_protocol(childout.getline());
}
// Either way, stop looping.
break;
}
// Saw ':', read length prefix and store in mExpect.
std::istream& childstream(childout.get_istream());
size_t expect;
childstream >> expect;
int colon(childstream.get());
if (colon != ':')
{
// Protocol failure. Clear out the rest of the pending data in
// childout (well, up to a max length) to log what was wrong.
LLProcess::ReadPipe::size_type
readlen((std::min)(childout.size(),
LLProcess::ReadPipe::size_type(80)));
bad_protocol(stringize(expect, char(colon), childout.read(readlen)));
break;
}
else
{
// Saw length prefix, saw colon, life is good. Now wait for
// that length of data to arrive.
mExpect = expect;
LL_DEBUGS("LLLeap") << "got length, waiting for "
<< mExpect << " bytes of data" << LL_ENDL;
// Transition to "read data" mode and loop back to check
// if we've already received all the advertised data.
mReadPrefix = false;
continue;
}
}
/*----------------- saw prefix, wait for data ------------------*/
else
{
// Saw length prefix, saw colon, life is good. Now wait for
// that length of data to arrive.
mExpect = expect;
LL_DEBUGS("LLLeap") << "got length, waiting for "
<< mExpect << " bytes of data" << LL_ENDL;
// Block calls to this method; resetting mBlocker unblocks
// calls to the other method.
mBlocker.reset(new LLEventPump::Blocker(mStdoutConnection));
// Go check if we've already received all the advertised data.
if (childout.size())
// Until we've accumulated the promised length of data, keep waiting.
if (childout.size() < mExpect)
{
LLSD updata(data);
updata["len"] = LLSD::Integer(childout.size());
rstdoutData(updata);
break;
}
}
}
else if (childout.contains('\n'))
{
// Since this is the initial listening state, this is where we'd
// arrive if the child isn't following protocol at all -- say
// because the user specified 'ls' or some darn thing.
bad_protocol(childout.getline());
}
return false;
}
// State in which we listen on stdout for the specified length of data to
// arrive.
bool rstdoutData(const LLSD& data)
{
LLProcess::ReadPipe& childout(mChild->getReadPipe(LLProcess::STDOUT));
// Until we've accumulated the promised length of data, keep waiting.
if (childout.size() >= mExpect)
{
// Ready to rock and roll.
LL_DEBUGS("LLLeap") << "needed " << mExpect << " bytes, got "
<< childout.size() << ", parsing LLSD" << LL_ENDL;
LLSD data;
// We have the data we were told to expect! Ready to rock and roll.
LL_DEBUGS("LLLeap") << "needed " << mExpect << " bytes, got "
<< childout.size() << ", parsing LLSD" << LL_ENDL;
LLSD data;
#if 1
// specifically require notation LLSD from child
LLPointer<LLSDParser> parser(new LLSDNotationParser());
S32 parse_status(parser->parse(childout.get_istream(), data, mExpect));
if (parse_status == LLSDParser::PARSE_FAILURE)
// specifically require notation LLSD from child
LLPointer<LLSDParser> parser(new LLSDNotationParser());
S32 parse_status(parser->parse(childout.get_istream(), data, mExpect));
if (parse_status == LLSDParser::PARSE_FAILURE)
#else
// SL-18330: accept any valid LLSD serialization format from child
// Unfortunately this runs into trouble we have not yet debugged.
bool parse_status(LLSDSerialize::deserialize(data, childout.get_istream(), mExpect));
if (! parse_status)
// SL-18330: accept any valid LLSD serialization format from child
// Unfortunately this runs into trouble we have not yet debugged.
bool parse_status(LLSDSerialize::deserialize(data, childout.get_istream(), mExpect));
if (! parse_status)
#endif
{
bad_protocol("unparseable LLSD data");
}
else if (! (data.isMap() && data["pump"].isString() && data.has("data")))
{
// we got an LLSD object, but it lacks required keys
bad_protocol("missing 'pump' or 'data'");
}
else
{
try
{
// The LLSD object we got from our stream contains the
// keys we need.
LLEventPumps::instance().obtain(data["pump"]).post(data["data"]);
bad_protocol("unparseable LLSD data");
break;
}
catch (const std::exception& err)
else if (! (data.isMap() && data["pump"].isString() && data.has("data")))
{
// No plugin should be allowed to crash the viewer by
// driving an exception -- intentionally or not.
LOG_UNHANDLED_EXCEPTION(stringize("handling request ", data));
// Whether or not the plugin added a "reply" key to the
// request, send a reply. We happen to know who originated
// this request, and the reply LLEventPump of interest.
// Not our problem if the plugin ignores the reply event.
data["reply"] = mReplyPump.getName();
sendReply(llsd::map("error",
stringize(LLError::Log::classname(err), ": ", err.what())),
data);
// we got an LLSD object, but it lacks required keys
bad_protocol("missing 'pump' or 'data'");
break;
}
// Block calls to this method; resetting mBlocker unblocks
// calls to the other method.
mBlocker.reset(new LLEventPump::Blocker(mStdoutDataConnection));
// Go check for any more pending events in the buffer.
if (childout.size())
else
{
LLSD updata(data);
data["len"] = LLSD::Integer(childout.size());
rstdout(updata);
try
{
// The LLSD object we got from our stream contains the
// keys we need.
LLEventPumps::instance().obtain(data["pump"]).post(data["data"]);
}
catch (const std::exception& err)
{
// No plugin should be allowed to crash the viewer by
// driving an exception -- intentionally or not.
LOG_UNHANDLED_EXCEPTION(stringize("handling request ", data));
// Whether or not the plugin added a "reply" key to the
// request, send a reply. We happen to know who originated
// this request, and the reply LLEventPump of interest.
// Not our problem if the plugin ignores the reply event.
data["reply"] = mReplyPump.getName();
sendReply(llsd::map("error",
stringize(LLError::Log::classname(err), ": ", err.what())),
data);
}
// Transition to "read prefix" mode and go check for any
// more pending events in the buffer.
mReadPrefix = true;
continue;
}
}
}
......@@ -453,7 +448,8 @@ class LLLeapImpl: public LLLeap
// child's stdin, suitably enriched with the pump name on which it was
// received.
return pump.listen(listener,
boost::bind(&LLLeapImpl::wstdin, this, pump.getName(), _1));
[this, name=pump.getName()](const LLSD& data)
{ return wstdin(name, data); });
}
std::string mDesc;
......@@ -461,11 +457,11 @@ class LLLeapImpl: public LLLeap
LLEventStream mReplyPump;
LLProcessPtr mChild;
LLTempBoundListener
mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection;
std::unique_ptr<LLEventPump::Blocker> mBlocker;
mStdinConnection, mStdoutConnection, mStderrConnection;
LLProcess::ReadPipe::size_type mExpect;
LLError::RecorderPtr mRecorder;
std::unique_ptr<LLLeapListener> mListener;
bool mReadPrefix;
};
// These must follow the declaration of LLLeapImpl, so they may as well be last.
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment