diff options
author | nat-goodspeed <nat@lindenlab.com> | 2024-02-23 11:39:24 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-02-23 11:39:24 -0500 |
commit | ad32c066691152e6a23f025d6aa5ead0e91b7be9 (patch) | |
tree | 95256b06398914fc8a91b35e60d7de9a78d90a02 /indra/llcommon | |
parent | b631f9aa218e56b12b4648a37f92ddf405eaa0f4 (diff) | |
parent | 904d82402c8b927f5e09830d10247079fb4a94f4 (diff) |
Merge pull request #879 from secondlife/lua-events
Lua listen_events(), await_event() => get_event_pumps(), get_event_next().
Diffstat (limited to 'indra/llcommon')
-rw-r--r-- | indra/llcommon/llevents.cpp | 8 | ||||
-rw-r--r-- | indra/llcommon/llleap.cpp | 61 | ||||
-rw-r--r-- | indra/llcommon/llleaplistener.cpp | 37 | ||||
-rw-r--r-- | indra/llcommon/llleaplistener.h | 27 | ||||
-rw-r--r-- | indra/llcommon/lua_function.cpp | 10 | ||||
-rw-r--r-- | indra/llcommon/lua_function.h | 12 | ||||
-rw-r--r-- | indra/llcommon/lualistener.cpp | 76 | ||||
-rw-r--r-- | indra/llcommon/lualistener.h | 35 | ||||
-rw-r--r-- | indra/llcommon/tests/lleventcoro_test.cpp | 20 |
9 files changed, 135 insertions, 151 deletions
diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index deb0776887..01bba7a620 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -43,6 +43,7 @@ #include <typeinfo> #include <cmath> #include <cctype> +#include <iomanip> // std::quoted // external library headers #include <boost/range/iterator_range.hpp> #if LL_WINDOWS @@ -189,8 +190,13 @@ bool LLEventPumps::post(const std::string&name, const LLSD&message) PumpMap::iterator found = mPumpMap.find(name); if (found == mPumpMap.end()) + { + LL_DEBUGS("LLEventPumps") << "LLEventPump(" << std::quoted(name) << ") not found" + << LL_ENDL; return false; + } +// LL_DEBUGS("LLEventPumps") << "posting to " << name << ": " << message << LL_ENDL; return (*found).second->post(message); } @@ -405,7 +411,7 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL { if (!mSignal) { - LL_WARNS() << "Can't connect listener" << LL_ENDL; + LL_WARNS("LLEventPump") << "Can't connect listener" << LL_ENDL; // connect will fail, return dummy return LLBoundListener(); } diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index 8f88e728ce..f7f2981638 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -14,13 +14,11 @@ // associated header #include "llleap.h" // STL headers -#include <sstream> #include <algorithm> +#include <memory> +#include <sstream> // 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" @@ -53,18 +51,19 @@ public: // We expect multiple LLLeapImpl instances. Definitely tweak // mDonePump's name for uniqueness. mDonePump("LLLeap", true), - // Troubling thought: what if one plugin intentionally messes with - // another plugin? LLEventPump names are in a single global namespace. - // Try to make that more difficult by generating a UUID for the reply- - // pump name -- so it should NOT need tweaking for uniqueness. - mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) - // Pass it a callback to our connect() method, so it can send events + // Pass it our wstdin() method as its callback, 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( + "LLLeap", + // Serialize any reply event to our child's stdin, suitably + // enriched with the pump name on which it was received. + [this](const std::string& pump, const LLSD& data) + { return wstdin(pump, data); })) { // Rule out unpopulated Params block if (! cparams.executable.isProvided()) @@ -93,7 +92,7 @@ public: } // 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 @@ public: // 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. @@ -123,17 +122,14 @@ public: childout.setLimit(20); childerr.setLimit(20); - // Serialize any event received on mReplyPump to our child's stdin. - mStdinConnection = connect(mReplyPump, "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. mStdoutConnection = childout.getPump() - .listen("prefix", boost::bind(&LLLeapImpl::rstdout, this, _1)); + .listen("prefix", [this](const LLSD& data){ return rstdout(data); }); mStdoutDataConnection = childout.getPump() - .listen("data", boost::bind(&LLLeapImpl::rstdoutData, this, _1)); + .listen("data", [this](const LLSD& data){ return rstdoutData(data); }); mBlocker.reset(new LLEventPump::Blocker(mStdoutDataConnection)); // Log anything sent up through stderr. When a typical program @@ -142,7 +138,7 @@ public: // 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( @@ -151,7 +147,7 @@ public: // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! - wstdin(mReplyPump.getName(), + wstdin(mListener->getReplyPump().getName(), LLSDMap ("command", mListener->getName()) // Include LLLeap features -- this may be important for child to @@ -198,7 +194,7 @@ public: return false; } - // Listener for events on mReplyPump: send to child stdin + // Listener for reply events: send to child stdin bool wstdin(const std::string& pump, const LLSD& data) { LLSD packet(LLSDMap("pump", pump)("data", data)); @@ -355,7 +351,7 @@ public: // 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(); + data["reply"] = mListener->getReplyPump().getName(); sendReply(llsd::map("error", stringize(LLError::Log::classname(err), ": ", err.what())), data); @@ -428,7 +424,7 @@ public: LLSD event; event["type"] = "error"; event["error"] = error; - mReplyPump.post(event); + mListener->getReplyPump().post(event); // All the above really accomplished was to buffer the serialized // event in our WritePipe. Have to pump mainloop a couple times to @@ -445,27 +441,14 @@ public: } private: - /// We always want to listen on mReplyPump with wstdin(); under some - /// circumstances we'll also echo other LLEventPumps to the plugin. - LLBoundListener connect(LLEventPump& pump, const std::string& listener) - { - // Serialize any event received on the specified LLEventPump to our - // 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)); - } - std::string mDesc; LLEventStream mDonePump; - LLEventStream mReplyPump; LLProcessPtr mChild; - LLTempBoundListener - mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; - boost::scoped_ptr<LLEventPump::Blocker> mBlocker; + LLTempBoundListener mStdoutConnection, mStdoutDataConnection, mStderrConnection; + std::unique_ptr<LLEventPump::Blocker> mBlocker; LLProcess::ReadPipe::size_type mExpect; LLError::RecorderPtr mRecorder; - boost::scoped_ptr<LLLeapListener> mListener; + std::unique_ptr<LLLeapListener> mListener; }; // These must follow the declaration of LLLeapImpl, so they may as well be last. diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index 471f52e91c..55e4752c5d 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -54,12 +54,19 @@ return features; } -LLLeapListener::LLLeapListener(const ConnectFunc& connect): +LLLeapListener::LLLeapListener(const std::string_view& caller, const Callback& callback): // Each LEAP plugin has an instance of this listener. Make the command // pump name difficult for other such plugins to guess. LLEventAPI(LLUUID::generateNewID().asString(), "Operations relating to the LLSD Event API Plugin (LEAP) protocol"), - mConnect(connect) + mCaller(caller), + mCallback(callback), + // Troubling thought: what if one plugin intentionally messes with + // another plugin? LLEventPump names are in a single global namespace. + // Try to make that more difficult by generating a UUID for the reply- + // pump name -- so it should NOT need tweaking for uniqueness. + mReplyPump(LLUUID::generateNewID().asString()), + mReplyConn(connect(mReplyPump, mCaller)) { LLSD need_name(LLSDMap("name", LLSD())); add("newpump", @@ -133,8 +140,7 @@ void LLLeapListener::newpump(const LLSD& request) reply["name"] = name; // Now listen on this new pump with our plugin listener - std::string myname("llleap"); - saveListener(name, myname, mConnect(new_pump, myname)); + saveListener(name, mCaller, connect(new_pump, mCaller)); } catch (const LLEventPumps::BadType& error) { @@ -170,7 +176,7 @@ void LLLeapListener::listen(const LLSD& request) { // "dest" unspecified means to direct events on "source" // to our plugin listener. - saveListener(source_name, listener_name, mConnect(source, listener_name)); + saveListener(source_name, listener_name, connect(source, listener_name)); } reply["status"] = true; } @@ -292,10 +298,27 @@ void LLLeapListener::getFeature(const LLSD& request) const } } +LLBoundListener LLLeapListener::connect(LLEventPump& pump, const std::string& listener) +{ + // Connect to source pump with an adapter that calls our callback with the + // pump name as well as the event data. + return pump.listen( + listener, + [callback=mCallback, pump=pump.getName()] + (const LLSD& data) + { return callback(pump, data); }); +} + void LLLeapListener::saveListener(const std::string& pump_name, const std::string& listener_name, const LLBoundListener& listener) { - mListeners.insert(ListenersMap::value_type(ListenersMap::key_type(pump_name, listener_name), - listener)); + // Don't use insert() or emplace() because, if this (pump_name, + // listener_name) pair is already in mListeners, we *want* to overwrite it. + auto& listener_entry{ mListeners[ListenersMap::key_type(pump_name, listener_name)] }; + // If we already stored a connection for this pump and listener name, + // disconnect it before overwriting it. But if this entry was newly + // created, disconnect() will be a no-op. + listener_entry.disconnect(); + listener_entry = listener; } diff --git a/indra/llcommon/llleaplistener.h b/indra/llcommon/llleaplistener.h index 0ca5893657..040fb737b7 100644 --- a/indra/llcommon/llleaplistener.h +++ b/indra/llcommon/llleaplistener.h @@ -13,10 +13,9 @@ #define LL_LLLEAPLISTENER_H #include "lleventapi.h" +#include <functional> #include <map> #include <string> -#include <boost/function.hpp> -#include <boost/ptr_container/ptr_map.hpp> /// Listener class implementing LLLeap query/control operations. /// See https://jira.lindenlab.com/jira/browse/DEV-31978. @@ -24,18 +23,16 @@ class LLLeapListener: public LLEventAPI { public: /** - * Decouple LLLeap by dependency injection. Certain LLLeapListener - * operations must be able to cause LLLeap to listen on a specified - * LLEventPump with the LLLeap listener that wraps incoming events in an - * outer (pump=, data=) map and forwards them to the plugin. Very well, - * define the signature for a function that will perform that, and make - * our constructor accept such a function. + * Certain LLLeapListener operations listen on a specified LLEventPump. + * Accept a bool(pump, data) callback from our caller for when any such + * event is received. */ - typedef boost::function<LLBoundListener(LLEventPump&, const std::string& listener)> - ConnectFunc; - LLLeapListener(const ConnectFunc& connect); + using Callback = std::function<bool(const std::string& pump, const LLSD& data)>; + LLLeapListener(const std::string_view& caller, const Callback& callback); ~LLLeapListener(); + LLEventPump& getReplyPump() { return mReplyPump; } + static LLSD getFeatures(); private: @@ -48,10 +45,16 @@ private: void getFeatures(const LLSD&) const; void getFeature(const LLSD&) const; + LLBoundListener connect(LLEventPump& pump, const std::string& listener); void saveListener(const std::string& pump_name, const std::string& listener_name, const LLBoundListener& listener); - ConnectFunc mConnect; + // The relative order of these next declarations is important because the + // constructor will initialize in this order. + std::string mCaller; + Callback mCallback; + LLEventStream mReplyPump; + LLTempBoundListener mReplyConn; // In theory, listen() could simply call the relevant LLEventPump's // listen() method, stoplistening() likewise. Lifespan issues make us diff --git a/indra/llcommon/lua_function.cpp b/indra/llcommon/lua_function.cpp index 13210fe5a6..906a3f379c 100644 --- a/indra/llcommon/lua_function.cpp +++ b/indra/llcommon/lua_function.cpp @@ -16,6 +16,7 @@ // STL headers // std headers #include <algorithm> +#include <iomanip> // std::quoted #include <map> #include <memory> // std::unique_ptr // external library headers @@ -92,7 +93,6 @@ LLSD lua_tollsd(lua_State* L, int index) { LL_DEBUGS("Lua") << "lua_tollsd(" << index << ") of " << lua_gettop(L) << " stack entries: " << lua_what(L, index) << LL_ENDL; - DebugExit log_exit("lua_tollsd()"); switch (lua_type(L, index)) { case LUA_TNONE: @@ -785,11 +785,3 @@ std::ostream& operator<<(std::ostream& out, const lua_stack& self) out << ']'; return out; } - -/***************************************************************************** -* DebugExit -*****************************************************************************/ -DebugExit::~DebugExit() -{ - LL_DEBUGS("Lua") << "exit " << mName << LL_ENDL; -} diff --git a/indra/llcommon/lua_function.h b/indra/llcommon/lua_function.h index 7973a769be..eccc92ec09 100644 --- a/indra/llcommon/lua_function.h +++ b/indra/llcommon/lua_function.h @@ -213,16 +213,4 @@ private: lua_State* L; }; -// log exit from any block declaring an instance of DebugExit, regardless of -// how control leaves that block -struct DebugExit -{ - DebugExit(const std::string& name): mName(name) {} - DebugExit(const DebugExit&) = delete; - DebugExit& operator=(const DebugExit&) = delete; - ~DebugExit(); - - std::string mName; -}; - #endif /* ! defined(LL_LUA_FUNCTION_H) */ diff --git a/indra/llcommon/lualistener.cpp b/indra/llcommon/lualistener.cpp index 0fa03ffb3b..ed34133924 100644 --- a/indra/llcommon/lualistener.cpp +++ b/indra/llcommon/lualistener.cpp @@ -24,20 +24,23 @@ #include "llleaplistener.h" #include "lua_function.h" -LuaListener::LuaListener(lua_State* L): - super(getUniqueKey()), - mListener( - new LLLeapListener( - [L](LLEventPump& pump, const std::string& listener) - { return connect(L, pump, listener); })) +const int MAX_QSIZE = 1000; + +std::ostream& operator<<(std::ostream& out, const LuaListener& self) { - mReplyConnection = connect(L, mReplyPump, "LuaListener"); + return out << "LuaListener(" << self.getReplyName() << ", " << self.getCommandName() << ")"; } +LuaListener::LuaListener(lua_State* L): + super(getUniqueKey()), + mListener(new LLLeapListener( + "LuaListener", + [this](const std::string& pump, const LLSD& data) + { return queueEvent(pump, data); })) +{} + LuaListener::~LuaListener() -{ - LL_DEBUGS("Lua") << "~LuaListener('" << mReplyPump.getName() << "')" << LL_ENDL; -} +{} int LuaListener::getUniqueKey() { @@ -54,50 +57,35 @@ int LuaListener::getUniqueKey() return key; } -LLBoundListener LuaListener::connect(lua_State* L, LLEventPump& pump, const std::string& listener) +std::string LuaListener::getReplyName() const { - return pump.listen( - listener, - [L, pumpname=pump.getName()](const LLSD& data) - { return call_lua(L, pumpname, data); }); + return mListener->getReplyPump().getName(); } -bool LuaListener::call_lua(lua_State* L, const std::string& pump, const LLSD& data) +std::string LuaListener::getCommandName() const { - LL_INFOS("Lua") << "LuaListener::call_lua('" << pump << "', " << data << ")" << LL_ENDL; - if (! lua_checkstack(L, 3)) - { - LL_WARNS("Lua") << "Cannot extend Lua stack to call listen_events() callback" - << LL_ENDL; - return false; - } - // push the registered Lua callback function stored in our registry as - // "event.function" - lua_getfield(L, LUA_REGISTRYINDEX, "event.function"); - llassert(lua_isfunction(L, -1)); - // pass pump name - lua_pushstdstring(L, pump); - // then the data blob - lua_pushllsd(L, data); - // call the registered Lua listener function; allow it to return bool; - // no message handler - auto status = lua_pcall(L, 2, 1, 0); - bool result{ false }; - if (status != LUA_OK) + return mListener->getPumpName(); +} + +bool LuaListener::queueEvent(const std::string& pump, const LLSD& data) +{ + // Our Lua script might be stalled, or just fail to retrieve events. Don't + // grow this queue indefinitely. But don't set MAX_QSIZE as the queue + // capacity or we'd block the post() call trying to propagate this event! + if (auto size = mQueue.size(); size > MAX_QSIZE) { - LL_WARNS("Lua") << "Error in listen_events() callback: " - << lua_tostdstring(L, -1) << LL_ENDL; + LL_WARNS("Lua") << "LuaListener queue for " << getReplyName() + << " exceeds " << MAX_QSIZE << ": " << size + << " -- discarding event" << LL_ENDL; } else { - result = lua_toboolean(L, -1); + mQueue.push(decltype(mQueue)::value_type(pump, data)); } - // discard either the error message or the bool return value - lua_pop(L, 1); - return result; + return false; } -std::string LuaListener::getCommandName() const +LuaListener::PumpData LuaListener::getNext() { - return mListener->getPumpName(); + return mQueue.pop(); } diff --git a/indra/llcommon/lualistener.h b/indra/llcommon/lualistener.h index df88d55dd5..c13b7bbd5f 100644 --- a/indra/llcommon/lualistener.h +++ b/indra/llcommon/lualistener.h @@ -12,14 +12,13 @@ #if ! defined(LL_LUALISTENER_H) #define LL_LUALISTENER_H -#include "llevents.h" #include "llinstancetracker.h" -#include "lluuid.h" +#include "llsd.h" +#include "llthreadsafequeue.h" +#include <iosfwd> // std::ostream #include <memory> // std::unique_ptr - -#ifdef LL_TEST -#include "lleventfilter.h" -#endif +#include <string> +#include <utility> // std::pair struct lua_State; class LLLeapListener; @@ -31,7 +30,6 @@ class LLLeapListener; * inconvenience malicious Lua scripts wanting to mess with others. The idea * is that a given lua_State stores in its Registry: * - "event.listener": the int key of the corresponding LuaListener, if any - * - "event.function": the Lua function to be called with incoming events * The original thought was that LuaListener would itself store the Lua * function -- but surprisingly, there is no C/C++ type in the API that stores * a Lua function. @@ -55,22 +53,25 @@ public: ~LuaListener(); - std::string getReplyName() const { return mReplyPump.getName(); } + std::string getReplyName() const; std::string getCommandName() const; + /** + * LuaListener enqueues reply events from its LLLeapListener on mQueue. + * Call getNext() to retrieve the next such event. Blocks the calling + * coroutine if the queue is empty. + */ + using PumpData = std::pair<std::string, LLSD>; + PumpData getNext(); + + friend std::ostream& operator<<(std::ostream& out, const LuaListener& self); + private: static int getUniqueKey(); + bool queueEvent(const std::string& pump, const LLSD& data); - static LLBoundListener connect(lua_State* L, LLEventPump& pump, const std::string& listener); - - static bool call_lua(lua_State* L, const std::string& pump, const LLSD& data); + LLThreadSafeQueue<PumpData> mQueue; -#ifndef LL_TEST - LLEventStream mReplyPump{ LLUUID::generateNewID().asString() }; -#else - LLEventLogProxyFor<LLEventStream> mReplyPump{ "luapump", false }; -#endif - LLTempBoundListener mReplyConnection; std::unique_ptr<LLLeapListener> mListener; }; diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index 032923a108..c7a958da49 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -113,7 +113,7 @@ namespace tut void test_data::explicit_wait(boost::shared_ptr<LLCoros::Promise<std::string>>& cbp) { - BEGIN + DEBUGIN { mSync.bump(); // The point of this test is to verify / illustrate suspending a @@ -136,7 +136,7 @@ namespace tut mSync.bump(); ensure_equals("Got it", stringdata, "received"); } - END + DEBUGEND } template<> template<> @@ -163,13 +163,13 @@ namespace tut void test_data::waitForEventOn1() { - BEGIN + DEBUGIN { mSync.bump(); result = suspendUntilEventOn("source"); mSync.bump(); } - END + DEBUGEND } template<> template<> @@ -189,7 +189,7 @@ namespace tut void test_data::coroPump() { - BEGIN + DEBUGIN { mSync.bump(); LLCoroEventPump waiter; @@ -197,7 +197,7 @@ namespace tut result = waiter.suspend(); mSync.bump(); } - END + DEBUGEND } template<> template<> @@ -217,7 +217,7 @@ namespace tut void test_data::postAndWait1() { - BEGIN + DEBUGIN { mSync.bump(); result = postAndSuspend(LLSDMap("value", 17), // request event @@ -226,7 +226,7 @@ namespace tut "reply"); // request["reply"] = name mSync.bump(); } - END + DEBUGEND } template<> template<> @@ -240,7 +240,7 @@ namespace tut void test_data::coroPumpPost() { - BEGIN + DEBUGIN { mSync.bump(); LLCoroEventPump waiter; @@ -248,7 +248,7 @@ namespace tut immediateAPI.getPump(), "reply"); mSync.bump(); } - END + DEBUGEND } template<> template<> |