From 4e7838fb004f67c51d1b9991ba6782be7036bd7e Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Fri, 26 Mar 2010 15:39:21 -0700 Subject: Implemented central storage mechanism for media plugin cookies. Added LLPluginCookieStore, which manages the central list of cookies. New Mac and Windows versions of llqtwebkit, built from the tip of the cookie-api branch on http://bitbucket.org/lindenlab/llqtwebkit/ (currently revision f35a5eab8c2f). Added "set_cookies" and "cookie_set" messages to the media_browser message class in the plugin API, and made the webkit plugin use them appropriately. Added methods to LLViewerMedia to read/write the cookie file and add/remove individual cookies. Added hooks to read/write the cookie file (plugin_cookies.txt) in the same places as the location history (idle_startup() in llstartup.cpp and LLAppViewer::cleanup(), respectively). Reviewed by Richard at http://codereview.lindenlab.com/1006003 --- indra/llplugin/CMakeLists.txt | 2 + indra/llplugin/llpluginclassmedia.cpp | 14 + indra/llplugin/llpluginclassmedia.h | 1 + indra/llplugin/llpluginclassmediaowner.h | 2 + indra/llplugin/llplugincookiestore.cpp | 600 +++++++++++++++++++++++++++++++ indra/llplugin/llplugincookiestore.h | 122 +++++++ 6 files changed, 741 insertions(+) create mode 100644 indra/llplugin/llplugincookiestore.cpp create mode 100644 indra/llplugin/llplugincookiestore.h (limited to 'indra/llplugin') diff --git a/indra/llplugin/CMakeLists.txt b/indra/llplugin/CMakeLists.txt index 6706775d4f..def9fcbeae 100644 --- a/indra/llplugin/CMakeLists.txt +++ b/indra/llplugin/CMakeLists.txt @@ -23,6 +23,7 @@ include_directories( set(llplugin_SOURCE_FILES llpluginclassmedia.cpp + llplugincookiestore.cpp llplugininstance.cpp llpluginmessage.cpp llpluginmessagepipe.cpp @@ -36,6 +37,7 @@ set(llplugin_HEADER_FILES llpluginclassmedia.h llpluginclassmediaowner.h + llplugincookiestore.h llplugininstance.h llpluginmessage.h llpluginmessageclasses.h diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index bf0e19473e..e09b511a6e 100644 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -993,6 +993,13 @@ void LLPluginClassMedia::receivePluginMessage(const LLPluginMessage &message) mClickTargetType = TARGET_NONE; mediaEvent(LLPluginClassMediaOwner::MEDIA_EVENT_CLICK_LINK_NOFOLLOW); } + else if(message_name == "cookie_set") + { + if(mOwner) + { + mOwner->handleCookieSet(this, message.getValue("cookie")); + } + } else { LL_WARNS("Plugin") << "Unknown " << message_name << " class message: " << message_name << LL_ENDL; @@ -1076,6 +1083,13 @@ void LLPluginClassMedia::clear_cookies() sendMessage(message); } +void LLPluginClassMedia::set_cookies(const std::string &cookies) +{ + LLPluginMessage message(LLPLUGIN_MESSAGE_CLASS_MEDIA_BROWSER, "set_cookies"); + message.setValue("cookies", cookies); + sendMessage(message); +} + void LLPluginClassMedia::enable_cookies(bool enable) { LLPluginMessage message(LLPLUGIN_MESSAGE_CLASS_MEDIA_BROWSER, "enable_cookies"); diff --git a/indra/llplugin/llpluginclassmedia.h b/indra/llplugin/llpluginclassmedia.h index 79356beb68..8c7b00f45b 100644 --- a/indra/llplugin/llpluginclassmedia.h +++ b/indra/llplugin/llpluginclassmedia.h @@ -189,6 +189,7 @@ public: void focus(bool focused); void clear_cache(); void clear_cookies(); + void set_cookies(const std::string &cookies); void enable_cookies(bool enable); void proxy_setup(bool enable, const std::string &host = LLStringUtil::null, int port = 0); void browse_stop(); diff --git a/indra/llplugin/llpluginclassmediaowner.h b/indra/llplugin/llpluginclassmediaowner.h index 6d369cd51a..5669b81fd1 100644 --- a/indra/llplugin/llpluginclassmediaowner.h +++ b/indra/llplugin/llpluginclassmediaowner.h @@ -39,6 +39,7 @@ #include class LLPluginClassMedia; +class LLPluginCookieStore; class LLPluginClassMediaOwner { @@ -78,6 +79,7 @@ public: virtual ~LLPluginClassMediaOwner() {}; virtual void handleMediaEvent(LLPluginClassMedia* /*self*/, EMediaEvent /*event*/) {}; + virtual void handleCookieSet(LLPluginClassMedia* /*self*/, const std::string &/*cookie*/) {}; }; #endif // LL_LLPLUGINCLASSMEDIAOWNER_H diff --git a/indra/llplugin/llplugincookiestore.cpp b/indra/llplugin/llplugincookiestore.cpp new file mode 100644 index 0000000000..1964b8d789 --- /dev/null +++ b/indra/llplugin/llplugincookiestore.cpp @@ -0,0 +1,600 @@ +/** + * @file llplugincookiestore.cpp + * @brief LLPluginCookieStore provides central storage for http cookies used by plugins + * + * @cond + * $LicenseInfo:firstyear=2010&license=viewergpl$ + * + * Copyright (c) 2010, Linden Research, Inc. + * + * Second Life Viewer Source Code + * The source code in this file ("Source Code") is provided by Linden Lab + * to you under the terms of the GNU General Public License, version 2.0 + * ("GPL"), unless you have obtained a separate licensing agreement + * ("Other License"), formally executed by you and Linden Lab. Terms of + * the GPL can be found in doc/GPL-license.txt in this distribution, or + * online at http://secondlife.com/developers/opensource/gplv2 + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution, or + * online at http://secondlife.com/developers/opensource/flossexception + * + * By copying, modifying or distributing this software, you acknowledge + * that you have read and understood your obligations described above, + * and agree to abide by those obligations. + * + * ALL LINDEN LAB SOURCE CODE IS PROVIDED "AS IS." LINDEN LAB MAKES NO + * WARRANTIES, EXPRESS, IMPLIED OR OTHERWISE, REGARDING ITS ACCURACY, + * COMPLETENESS OR PERFORMANCE. + * $/LicenseInfo$ + * @endcond + */ + +#include "linden_common.h" +#include "indra_constants.h" + +#include "llplugincookiestore.h" +#include + +// for curl_getdate() (apparently parsing RFC 1123 dates is hard) +#include + +LLPluginCookieStore::LLPluginCookieStore(): + mHasChangedCookies(false) +{ +} + + +LLPluginCookieStore::~LLPluginCookieStore() +{ + clearCookies(); +} + + +LLPluginCookieStore::Cookie::Cookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end): + mCookie(s, cookie_start, cookie_end), + mNameStart(0), mNameEnd(0), + mValueStart(0), mValueEnd(0), + mDomainStart(0), mDomainEnd(0), + mPathStart(0), mPathEnd(0), + mDead(false), mChanged(true) +{ +} + +LLPluginCookieStore::Cookie *LLPluginCookieStore::Cookie::createFromString(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end) +{ + Cookie *result = new Cookie(s, cookie_start, cookie_end); + + if(!result->parse()) + { + delete result; + result = NULL; + } + + return result; +} + +std::string LLPluginCookieStore::Cookie::getKey() const +{ + std::string result; + if(mDomainEnd > mDomainStart) + { + result += mCookie.substr(mDomainStart, mDomainEnd - mDomainStart); + } + result += ';'; + if(mPathEnd > mPathStart) + { + result += mCookie.substr(mPathStart, mPathEnd - mPathStart); + } + result += ';'; + result += mCookie.substr(mNameStart, mNameEnd - mNameStart); + return result; +} + +bool LLPluginCookieStore::Cookie::parse() +{ + bool first_field = true; + + std::string::size_type cookie_end = mCookie.size(); + std::string::size_type field_start = 0; + + lldebugs << "parsing cookie: " << mCookie << llendl; + while(field_start < cookie_end) + { + // Finding the start of the next field requires honoring special quoting rules + // see the definition of 'quoted-string' in rfc2616 for details + std::string::size_type next_field_start = findFieldEnd(field_start); + + // The end of this field should not include the terminating ';' or any trailing whitespace + std::string::size_type field_end = mCookie.find_last_not_of("; ", next_field_start); + if(field_end == std::string::npos || field_end < field_start) + { + // This field was empty or all whitespace. Set end = start so it shows as empty. + field_end = field_start; + } + else if (field_end < next_field_start) + { + // we actually want the index of the char _after_ what 'last not of' found + ++field_end; + } + + // find the start of the actual name (skip separator and possible whitespace) + std::string::size_type name_start = mCookie.find_first_not_of("; ", field_start); + if(name_start == std::string::npos || name_start > next_field_start) + { + // Again, nothing but whitespace. + name_start = field_start; + } + + // the name and value are separated by the first equals sign + std::string::size_type name_value_sep = mCookie.find_first_of("=", name_start); + if(name_value_sep == std::string::npos || name_value_sep > field_end) + { + // No separator found, so this is a field without an = + name_value_sep = field_end; + } + + // the name end is before the name-value separator + std::string::size_type name_end = mCookie.find_last_not_of("= ", name_value_sep); + if(name_end == std::string::npos || name_end < name_start) + { + // I'm not sure how we'd hit this case... it seems like it would have to be an empty name. + name_end = name_start; + } + else if (name_end < name_value_sep) + { + // we actually want the index of the char _after_ what 'last not of' found + ++name_end; + } + + // Value is between the name-value sep and the end of the field. + std::string::size_type value_start = mCookie.find_first_not_of("= ", name_value_sep); + if(value_start == std::string::npos || value_start > field_end) + { + // All whitespace or empty value + value_start = field_end; + } + std::string::size_type value_end = mCookie.find_last_not_of("; ", field_end); + if(value_end == std::string::npos || value_end < value_start) + { + // All whitespace or empty value + value_end = value_start; + } + else if (value_end < field_end) + { + // we actually want the index of the char _after_ what 'last not of' found + ++value_end; + } + + lldebugs + << " field name: \"" << mCookie.substr(name_start, name_end - name_start) + << "\", value: \"" << mCookie.substr(value_start, value_end - value_start) << "\"" + << llendl; + + // See whether this field is one we know + if(first_field) + { + // The first field is the name=value pair + mNameStart = name_start; + mNameEnd = name_end; + mValueStart = value_start; + mValueEnd = value_end; + first_field = false; + } + else + { + // Subsequent fields must come from the set in rfc2109 + if(matchName(name_start, name_end, "expires")) + { + std::string date_string(mCookie, value_start, value_end - value_start); + // If the cookie contains an "expires" field, it MUST contain a parsable date. + + // HACK: LLDate apparently can't PARSE an rfc1123-format date, even though it can GENERATE one. + // The curl function curl_getdate can do this, but I'm hesitant to unilaterally introduce a curl dependency in LLDate. +#if 1 + time_t date = curl_getdate(date_string.c_str(), NULL ); + mDate.secondsSinceEpoch((F64)date); + lldebugs << " expire date parsed to: " << mDate.asRFC1123() << llendl; +#else + // This doesn't work (rfc1123-format dates cause it to fail) + if(!mDate.fromString(date_string)) + { + // Date failed to parse. + llwarns << "failed to parse cookie's expire date: " << date << llendl; + return false; + } +#endif + } + else if(matchName(name_start, name_end, "domain")) + { + mDomainStart = value_start; + mDomainEnd = value_end; + } + else if(matchName(name_start, name_end, "path")) + { + mPathStart = value_start; + mPathEnd = value_end; + } + else if(matchName(name_start, name_end, "max-age")) + { + // TODO: how should we handle this? + } + else if(matchName(name_start, name_end, "secure")) + { + // We don't care about the value of this field (yet) + } + else if(matchName(name_start, name_end, "version")) + { + // We don't care about the value of this field (yet) + } + else if(matchName(name_start, name_end, "comment")) + { + // We don't care about the value of this field (yet) + } + else + { + // An unknown field is a parse failure + return false; + } + + } + + + // move on to the next field, skipping this field's separator and any leading whitespace + field_start = mCookie.find_first_not_of("; ", next_field_start); + } + + // The cookie MUST have a name + if(mNameEnd <= mNameStart) + return false; + + return true; +} + +std::string::size_type LLPluginCookieStore::Cookie::findFieldEnd(std::string::size_type start, std::string::size_type end) +{ + std::string::size_type result = start; + + if(end == std::string::npos) + end = mCookie.size(); + + bool in_quotes = false; + for(; (result < end); result++) + { + switch(mCookie[result]) + { + case '\\': + if(in_quotes) + result++; // The next character is backslash-quoted. Skip over it. + break; + case '"': + in_quotes = !in_quotes; + break; + case ';': + if(!in_quotes) + return result; + break; + } + } + + // If we got here, no ';' was found. + return end; +} + +bool LLPluginCookieStore::Cookie::matchName(std::string::size_type start, std::string::size_type end, const char *name) +{ + // NOTE: this assumes 'name' is already in lowercase. The code which uses it should be able to arrange this... + + while((start < end) && (*name != '\0')) + { + if(tolower(mCookie[start]) != *name) + return false; + + start++; + name++; + } + + // iff both strings hit the end at the same time, they're equal. + return ((start == end) && (*name == '\0')); +} + +std::string LLPluginCookieStore::getAllCookies() +{ + std::stringstream result; + writeAllCookies(result); + return result.str(); +} + +void LLPluginCookieStore::writeAllCookies(std::ostream& s) +{ + cookie_map_t::iterator iter; + for(iter = mCookies.begin(); iter != mCookies.end(); iter++) + { + // Don't return expired cookies + if(!iter->second->isDead()) + { + s << (iter->second->getCookie()) << "\n"; + } + } + +} + +std::string LLPluginCookieStore::getPersistentCookies() +{ + std::stringstream result; + writePersistentCookies(result); + return result.str(); +} + +void LLPluginCookieStore::writePersistentCookies(std::ostream& s) +{ + cookie_map_t::iterator iter; + for(iter = mCookies.begin(); iter != mCookies.end(); iter++) + { + // Don't return expired cookies or session cookies + if(!iter->second->isDead() && !iter->second->isSessionCookie()) + { + s << iter->second->getCookie() << "\n"; + } + } +} + +std::string LLPluginCookieStore::getChangedCookies(bool clear_changed) +{ + std::stringstream result; + writeChangedCookies(result, clear_changed); + + return result.str(); +} + +void LLPluginCookieStore::writeChangedCookies(std::ostream& s, bool clear_changed) +{ + if(mHasChangedCookies) + { + lldebugs << "returning changed cookies: " << llendl; + cookie_map_t::iterator iter; + for(iter = mCookies.begin(); iter != mCookies.end(); ) + { + cookie_map_t::iterator next = iter; + next++; + + // Only return cookies marked as "changed" + if(iter->second->isChanged()) + { + s << iter->second->getCookie() << "\n"; + + lldebugs << " " << iter->second->getCookie() << llendl; + + // If requested, clear the changed mark + if(clear_changed) + { + if(iter->second->isDead()) + { + // If this cookie was previously marked dead, it needs to be removed entirely. + delete iter->second; + mCookies.erase(iter); + } + else + { + // Not dead, just mark as not changed. + iter->second->setChanged(false); + } + } + } + + iter = next; + } + } + + if(clear_changed) + mHasChangedCookies = false; +} + +void LLPluginCookieStore::setAllCookies(const std::string &cookies, bool mark_changed) +{ + clearCookies(); + setCookies(cookies, mark_changed); +} + +void LLPluginCookieStore::readAllCookies(std::istream& s, bool mark_changed) +{ + clearCookies(); + readCookies(s, mark_changed); +} + +void LLPluginCookieStore::setCookies(const std::string &cookies, bool mark_changed) +{ + std::string::size_type start = 0; + + while(start != std::string::npos) + { + std::string::size_type end = cookies.find('\n', start); + if(end > start) + { + // The line is non-empty. Try to create a cookie from it. + setOneCookie(cookies, start, end, mark_changed); + } + start = cookies.find_first_not_of("\n ", end); + } +} + +void LLPluginCookieStore::readCookies(std::istream& s, bool mark_changed) +{ + std::string line; + while(s.good() && !s.eof()) + { + std::getline(s, line); + if(!line.empty()) + { + // Try to create a cookie from this line. + setOneCookie(line, 0, std::string::npos, mark_changed); + } + } +} + +std::string LLPluginCookieStore::quoteString(const std::string &s) +{ + std::stringstream result; + + result << '"'; + + for(std::string::size_type i = 0; i < s.size(); ++i) + { + char c = s[i]; + switch(c) + { + // All these separators need to be quoted in HTTP headers, according to section 2.2 of rfc 2616: + case '(': case ')': case '<': case '>': case '@': + case ',': case ';': case ':': case '\\': case '"': + case '/': case '[': case ']': case '?': case '=': + case '{': case '}': case ' ': case '\t': + result << '\\'; + break; + } + + result << c; + } + + result << '"'; + + return result.str(); +} + +std::string LLPluginCookieStore::unquoteString(const std::string &s) +{ + std::stringstream result; + + bool in_quotes = false; + + for(std::string::size_type i = 0; i < s.size(); ++i) + { + char c = s[i]; + switch(c) + { + case '\\': + if(in_quotes) + { + // The next character is backslash-quoted. Pass it through untouched. + ++i; + if(i < s.size()) + { + result << s[i]; + } + continue; + } + break; + case '"': + in_quotes = !in_quotes; + continue; + break; + } + + result << c; + } + + return result.str(); +} + +// The flow for deleting a cookie is non-obvious enough that I should call it out here... +// Deleting a cookie is done by setting a cookie with the same name, path, and domain, but with an expire timestamp in the past. +// (This is exactly how a web server tells a browser to delete a cookie.) +// When deleting with mark_changed set to true, this replaces the existing cookie in the list with an entry that's marked both dead and changed. +// Some time later when writeChangedCookies() is called with clear_changed set to true, the dead cookie is deleted from the list after being returned, so that the +// delete operation (in the form of the expired cookie) is passed along. +void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed) +{ + Cookie *cookie = Cookie::createFromString(s, cookie_start, cookie_end); + if(cookie) + { + lldebugs << "setting cookie: " << cookie->getCookie() << llendl; + + // Create a key for this cookie + std::string key = cookie->getKey(); + + // Check to see whether this cookie should have expired + if(!cookie->isSessionCookie() && (cookie->getDate() < LLDate::now())) + { + // This cookie has expired. + if(mark_changed) + { + // If we're marking cookies as changed, we should keep it anyway since we'll need to send it out with deltas. + cookie->setDead(true); + lldebugs << " marking dead" << llendl; + } + else + { + // If we're not marking cookies as changed, we don't need to keep this cookie at all. + // If the cookie was already in the list, delete it. + removeCookie(key); + + delete cookie; + cookie = NULL; + + lldebugs << " removing" << llendl; + } + } + + if(cookie) + { + // If it already exists in the map, replace it. + cookie_map_t::iterator iter = mCookies.find(key); + if(iter != mCookies.end()) + { + if(iter->second->getCookie() == cookie->getCookie()) + { + // The new cookie is identical to the old -- don't mark as changed. + // Just leave the old one in the map. + delete cookie; + cookie = NULL; + + lldebugs << " unchanged" << llendl; + } + else + { + // A matching cookie was already in the map. Replace it. + delete iter->second; + iter->second = cookie; + + cookie->setChanged(mark_changed); + if(mark_changed) + mHasChangedCookies = true; + + lldebugs << " replacing" << llendl; + } + } + else + { + // The cookie wasn't in the map. Insert it. + mCookies.insert(std::make_pair(key, cookie)); + + cookie->setChanged(mark_changed); + if(mark_changed) + mHasChangedCookies = true; + + lldebugs << " adding" << llendl; + } + } + } +} + +void LLPluginCookieStore::clearCookies() +{ + while(!mCookies.empty()) + { + cookie_map_t::iterator iter = mCookies.begin(); + delete iter->second; + mCookies.erase(iter); + } +} + +void LLPluginCookieStore::removeCookie(const std::string &key) +{ + cookie_map_t::iterator iter = mCookies.find(key); + if(iter != mCookies.end()) + { + delete iter->second; + mCookies.erase(iter); + } +} + diff --git a/indra/llplugin/llplugincookiestore.h b/indra/llplugin/llplugincookiestore.h new file mode 100644 index 0000000000..5250f008b6 --- /dev/null +++ b/indra/llplugin/llplugincookiestore.h @@ -0,0 +1,122 @@ +/** + * @file llplugincookiestore.h + * @brief LLPluginCookieStore provides central storage for http cookies used by plugins + * + * @cond + * $LicenseInfo:firstyear=2010&license=viewergpl$ + * + * Copyright (c) 2010, Linden Research, Inc. + * + * Second Life Viewer Source Code + * The source code in this file ("Source Code") is provided by Linden Lab + * to you under the terms of the GNU General Public License, version 2.0 + * ("GPL"), unless you have obtained a separate licensing agreement + * ("Other License"), formally executed by you and Linden Lab. Terms of + * the GPL can be found in doc/GPL-license.txt in this distribution, or + * online at http://secondlife.com/developers/opensource/gplv2 + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution, or + * online at http://secondlife.com/developers/opensource/flossexception + * + * By copying, modifying or distributing this software, you acknowledge + * that you have read and understood your obligations described above, + * and agree to abide by those obligations. + * + * ALL LINDEN LAB SOURCE CODE IS PROVIDED "AS IS." LINDEN LAB MAKES NO + * WARRANTIES, EXPRESS, IMPLIED OR OTHERWISE, REGARDING ITS ACCURACY, + * COMPLETENESS OR PERFORMANCE. + * $/LicenseInfo$ + * @endcond + */ + +#ifndef LL_LLPLUGINCOOKIESTORE_H +#define LL_LLPLUGINCOOKIESTORE_H + +#include "lldate.h" +#include +#include +#include + +class LLPluginCookieStore +{ + LOG_CLASS(LLPluginCookieStore); +public: + LLPluginCookieStore(); + ~LLPluginCookieStore(); + + // gets all cookies currently in storage -- use when initializing a plugin + std::string getAllCookies(); + void writeAllCookies(std::ostream& s); + + // gets only persistent cookies (i.e. not session cookies) -- use when writing cookies to a file + std::string getPersistentCookies(); + void writePersistentCookies(std::ostream& s); + + // gets cookies which are marked as "changed" -- use when sending periodic updates to plugins + std::string getChangedCookies(bool clear_changed = true); + void writeChangedCookies(std::ostream& s, bool clear_changed = true); + + // (re)initializes internal data structures and bulk-sets cookies -- use when reading cookies from a file + void setAllCookies(const std::string &cookies, bool mark_changed = false); + void readAllCookies(std::istream& s, bool mark_changed = false); + + // sets one or more cookies (without reinitializing anything) -- use when receiving cookies from a plugin + void setCookies(const std::string &cookies, bool mark_changed = true); + void readCookies(std::istream& s, bool mark_changed = true); + + // quote or unquote a string as per the definition of 'quoted-string' in rfc2616 + static std::string quoteString(const std::string &s); + static std::string unquoteString(const std::string &s); + +private: + + void setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed); + + class Cookie + { + public: + static Cookie *createFromString(const std::string &s, std::string::size_type cookie_start = 0, std::string::size_type cookie_end = std::string::npos); + + // Construct a string from the cookie that uniquely represents it, to be used as a key in a std::map. + std::string getKey() const; + + const std::string &getCookie() const { return mCookie; }; + bool isSessionCookie() const { return mDate.isNull(); }; + + bool isDead() const { return mDead; }; + void setDead(bool dead) { mDead = dead; }; + + bool isChanged() const { return mChanged; }; + void setChanged(bool changed) { mChanged = changed; }; + + const LLDate &getDate() const { return mDate; }; + + private: + Cookie(const std::string &s, std::string::size_type cookie_start = 0, std::string::size_type cookie_end = std::string::npos); + bool parse(); + std::string::size_type findFieldEnd(std::string::size_type start = 0, std::string::size_type end = std::string::npos); + bool matchName(std::string::size_type start, std::string::size_type end, const char *name); + + std::string mCookie; // The full cookie, in RFC 2109 string format + LLDate mDate; // The expiration date of the cookie. For session cookies, this will be a null date (mDate.isNull() is true). + // Start/end indices of various parts of the cookie string. Stored as indices into the string to save space and time. + std::string::size_type mNameStart, mNameEnd; + std::string::size_type mValueStart, mValueEnd; + std::string::size_type mDomainStart, mDomainEnd; + std::string::size_type mPathStart, mPathEnd; + bool mDead; + bool mChanged; + }; + + typedef std::map cookie_map_t; + + cookie_map_t mCookies; + bool mHasChangedCookies; + + void clearCookies(); + void removeCookie(const std::string &key); +}; + +#endif // LL_LLPLUGINCOOKIESTORE_H -- cgit v1.2.3 From 5c190996222ce2cf14997e16e16beb110173c0a2 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 31 Mar 2010 13:53:40 -0700 Subject: Enable OpenID auth in the embedded webkit browser. Extract openid_url and openid_token tokens from the login response in process_login_success_response() and send them to LLViewerMedia if they're present. Added LLViewerMedia::openIDSetup() to receive openid_url and openid_token, and added code to LLViewerMedia to do a POST with LLHTTPClient, retrieve the resulting cookie, and push it into the central cookie store. Also made sure the OpenID cookie gets re-added when the cookie store is cleared. Added LLPluginCookieStore::setCookiesFromHost() to properly add a cookie that may not have a domain set. Made LLPluginCookieStore::Cookie::parse() add missing domain and path fields to cookies as necessary. Fixed an issue where carriage returns in the string passed to LLPluginCookieStore::setCookies() or LLPluginCookieStore::setCookiesFromHost() would cause a parse failure. Reviewed by gino and callum at http://codereview.lindenlab.com/1254001 --- indra/llplugin/llplugincookiestore.cpp | 73 ++++++++++++++++++++++++++++++---- indra/llplugin/llplugincookiestore.h | 9 +++-- 2 files changed, 72 insertions(+), 10 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llplugincookiestore.cpp b/indra/llplugin/llplugincookiestore.cpp index 1964b8d789..92ee24e1d5 100644 --- a/indra/llplugin/llplugincookiestore.cpp +++ b/indra/llplugin/llplugincookiestore.cpp @@ -62,11 +62,11 @@ LLPluginCookieStore::Cookie::Cookie(const std::string &s, std::string::size_type { } -LLPluginCookieStore::Cookie *LLPluginCookieStore::Cookie::createFromString(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end) +LLPluginCookieStore::Cookie *LLPluginCookieStore::Cookie::createFromString(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, const std::string &host) { Cookie *result = new Cookie(s, cookie_start, cookie_end); - if(!result->parse()) + if(!result->parse(host)) { delete result; result = NULL; @@ -92,7 +92,7 @@ std::string LLPluginCookieStore::Cookie::getKey() const return result; } -bool LLPluginCookieStore::Cookie::parse() +bool LLPluginCookieStore::Cookie::parse(const std::string &host) { bool first_field = true; @@ -248,7 +248,50 @@ bool LLPluginCookieStore::Cookie::parse() // The cookie MUST have a name if(mNameEnd <= mNameStart) return false; + + // If the cookie doesn't have a domain, add the current host as the domain. + if(mDomainEnd <= mDomainStart) + { + if(host.empty()) + { + // no domain and no current host -- this is a parse failure. + return false; + } + + // Figure out whether this cookie ended with a ";" or not... + std::string::size_type last_char = mCookie.find_last_not_of(" "); + if((last_char != std::string::npos) && (mCookie[last_char] != ';')) + { + mCookie += ";"; + } + + mCookie += " domain="; + mDomainStart = mCookie.size(); + mCookie += host; + mDomainEnd = mCookie.size(); + + lldebugs << "added domain (" << mDomainStart << " to " << mDomainEnd << "), new cookie is: " << mCookie << llendl; + } + + // If the cookie doesn't have a path, add "/". + if(mPathEnd <= mPathStart) + { + // Figure out whether this cookie ended with a ";" or not... + std::string::size_type last_char = mCookie.find_last_not_of(" "); + if((last_char != std::string::npos) && (mCookie[last_char] != ';')) + { + mCookie += ";"; + } + + mCookie += " path="; + mPathStart = mCookie.size(); + mCookie += "/"; + mPathEnd = mCookie.size(); + lldebugs << "added path (" << mPathStart << " to " << mPathEnd << "), new cookie is: " << mCookie << llendl; + } + + return true; } @@ -409,13 +452,29 @@ void LLPluginCookieStore::setCookies(const std::string &cookies, bool mark_chang while(start != std::string::npos) { - std::string::size_type end = cookies.find('\n', start); + std::string::size_type end = cookies.find_first_of("\r\n", start); if(end > start) { // The line is non-empty. Try to create a cookie from it. setOneCookie(cookies, start, end, mark_changed); } - start = cookies.find_first_not_of("\n ", end); + start = cookies.find_first_not_of("\r\n ", end); + } +} + +void LLPluginCookieStore::setCookiesFromHost(const std::string &cookies, const std::string &host, bool mark_changed) +{ + std::string::size_type start = 0; + + while(start != std::string::npos) + { + std::string::size_type end = cookies.find_first_of("\r\n", start); + if(end > start) + { + // The line is non-empty. Try to create a cookie from it. + setOneCookie(cookies, start, end, mark_changed, host); + } + start = cookies.find_first_not_of("\r\n ", end); } } @@ -502,9 +561,9 @@ std::string LLPluginCookieStore::unquoteString(const std::string &s) // When deleting with mark_changed set to true, this replaces the existing cookie in the list with an entry that's marked both dead and changed. // Some time later when writeChangedCookies() is called with clear_changed set to true, the dead cookie is deleted from the list after being returned, so that the // delete operation (in the form of the expired cookie) is passed along. -void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed) +void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed, const std::string &host) { - Cookie *cookie = Cookie::createFromString(s, cookie_start, cookie_end); + Cookie *cookie = Cookie::createFromString(s, cookie_start, cookie_end, host); if(cookie) { lldebugs << "setting cookie: " << cookie->getCookie() << llendl; diff --git a/indra/llplugin/llplugincookiestore.h b/indra/llplugin/llplugincookiestore.h index 5250f008b6..a93f0c14f0 100644 --- a/indra/llplugin/llplugincookiestore.h +++ b/indra/llplugin/llplugincookiestore.h @@ -66,18 +66,21 @@ public: void setCookies(const std::string &cookies, bool mark_changed = true); void readCookies(std::istream& s, bool mark_changed = true); + // sets one or more cookies (without reinitializing anything), supplying a hostname the cookies came from -- use when setting a cookie manually + void setCookiesFromHost(const std::string &cookies, const std::string &host, bool mark_changed = true); + // quote or unquote a string as per the definition of 'quoted-string' in rfc2616 static std::string quoteString(const std::string &s); static std::string unquoteString(const std::string &s); private: - void setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed); + void setOneCookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end, bool mark_changed, const std::string &host = LLStringUtil::null); class Cookie { public: - static Cookie *createFromString(const std::string &s, std::string::size_type cookie_start = 0, std::string::size_type cookie_end = std::string::npos); + static Cookie *createFromString(const std::string &s, std::string::size_type cookie_start = 0, std::string::size_type cookie_end = std::string::npos, const std::string &host = LLStringUtil::null); // Construct a string from the cookie that uniquely represents it, to be used as a key in a std::map. std::string getKey() const; @@ -95,7 +98,7 @@ private: private: Cookie(const std::string &s, std::string::size_type cookie_start = 0, std::string::size_type cookie_end = std::string::npos); - bool parse(); + bool parse(const std::string &host); std::string::size_type findFieldEnd(std::string::size_type start = 0, std::string::size_type end = std::string::npos); bool matchName(std::string::size_type start, std::string::size_type end, const char *name); -- cgit v1.2.3 From 11b588c29c5c49af70f4a55258bb934cb7ccdb55 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 31 Mar 2010 18:50:20 -0700 Subject: Fixed a problem with LLPluginCookieStore that would have caused problems when adding multiple cookies with setCookies(), setAllCookies(), and setCookiesFromHost(). --- indra/llplugin/llplugincookiestore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llplugincookiestore.cpp b/indra/llplugin/llplugincookiestore.cpp index 92ee24e1d5..85b1e70d78 100644 --- a/indra/llplugin/llplugincookiestore.cpp +++ b/indra/llplugin/llplugincookiestore.cpp @@ -53,7 +53,7 @@ LLPluginCookieStore::~LLPluginCookieStore() LLPluginCookieStore::Cookie::Cookie(const std::string &s, std::string::size_type cookie_start, std::string::size_type cookie_end): - mCookie(s, cookie_start, cookie_end), + mCookie(s, cookie_start, cookie_end - cookie_start), mNameStart(0), mNameEnd(0), mValueStart(0), mValueEnd(0), mDomainStart(0), mDomainEnd(0), -- cgit v1.2.3 From e868159922b97c5f7de4bdd83de65e135d16ff01 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 31 Mar 2010 18:51:09 -0700 Subject: Added unit test for LLPluginCookieStore. --- indra/llplugin/CMakeLists.txt | 17 +++ indra/llplugin/tests/llplugincookiestore_test.cpp | 178 ++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 indra/llplugin/tests/llplugincookiestore_test.cpp (limited to 'indra/llplugin') diff --git a/indra/llplugin/CMakeLists.txt b/indra/llplugin/CMakeLists.txt index def9fcbeae..441becbae0 100644 --- a/indra/llplugin/CMakeLists.txt +++ b/indra/llplugin/CMakeLists.txt @@ -3,6 +3,7 @@ project(llplugin) include(00-Common) +include(CURL) include(LLCommon) include(LLImage) include(LLMath) @@ -55,3 +56,19 @@ list(APPEND llplugin_SOURCE_FILES ${llplugin_HEADER_FILES}) add_library (llplugin ${llplugin_SOURCE_FILES}) add_subdirectory(slplugin) + +# Add tests +include(LLAddBuildTest) +# UNIT TESTS +SET(llplugin_TEST_SOURCE_FILES + llplugincookiestore.cpp + ) + +# llplugincookiestore has a dependency on curl, so we need to link the curl library into the test. +set_source_files_properties( + llplugincookiestore.cpp + PROPERTIES + LL_TEST_ADDITIONAL_LIBRARIES "${CURL_LIBRARIES}" + ) + +LL_ADD_PROJECT_UNIT_TESTS(llplugin "${llplugin_TEST_SOURCE_FILES}") diff --git a/indra/llplugin/tests/llplugincookiestore_test.cpp b/indra/llplugin/tests/llplugincookiestore_test.cpp new file mode 100644 index 0000000000..e3e8ac9804 --- /dev/null +++ b/indra/llplugin/tests/llplugincookiestore_test.cpp @@ -0,0 +1,178 @@ +#include "linden_common.h" +#include "../test/lltut.h" + +#include "../llplugincookiestore.h" + + +namespace tut +{ + // Main Setup + struct LLPluginCookieStoreFixture + { + LLPluginCookieStoreFixture() + { + // We need dates definitively in the past and the future to properly test cookie expiration. + LLDate now = LLDate::now(); + LLDate past(now.secondsSinceEpoch() - (60.0 * 60.0 * 24.0)); // 1 day in the past + LLDate future(now.secondsSinceEpoch() + (60.0 * 60.0 * 24.0)); // 1 day in the future + + mPastString = past.asRFC1123(); + mFutureString = future.asRFC1123(); + } + + std::string mPastString; + std::string mFutureString; + LLPluginCookieStore mCookieStore; + + // List of cookies used for validation + std::list mCookies; + + // This sets up mCookies from a string returned by one of the functions in LLPluginCookieStore + void setCookies(const std::string &cookies) + { + mCookies.clear(); + std::string::size_type start = 0; + + while(start != std::string::npos) + { + std::string::size_type end = cookies.find_first_of("\r\n", start); + if(end > start) + { + std::string line(cookies, start, end - start); + if(line.find_first_not_of("\r\n\t ") != std::string::npos) + { + // The line has some non-whitespace characters. Save it to the list. + mCookies.push_back(std::string(cookies, start, end - start)); + } + } + start = cookies.find_first_not_of("\r\n ", end); + } + } + + // This ensures that a cookie matching the one passed is in the list. + void ensureCookie(const std::string &cookie) + { + std::list::iterator iter; + for(iter = mCookies.begin(); iter != mCookies.end(); iter++) + { + if(*iter == cookie) + { + // Found the cookie + // TODO: this should do a smarter equality comparison on the two cookies, instead of just a string compare. + return; + } + } + + // Didn't find this cookie + std::string message = "cookie not found: "; + message += cookie; + ensure(message, false); + } + + // This ensures that the number of cookies in the list matches what's expected. + void ensureSize(const std::string &message, size_t size) + { + if(mCookies.size() != size) + { + std::stringstream full_message; + + full_message << message << " (expected " << size << ", actual " << mCookies.size() << ")"; + ensure(full_message.str(), false); + } + } + }; + + typedef test_group factory; + typedef factory::object object; + factory tf("LLPluginCookieStore test"); + + // Tests + template<> template<> + void object::test<1>() + { + // Test 1: cookie uniqueness and update lists. + // Valid, distinct cookies: + + std::string cookie01 = "cookieA=value; domain=example.com; path=/"; + std::string cookie02 = "cookieB=value; domain=example.com; path=/"; // different name + std::string cookie03 = "cookieA=value; domain=foo.example.com; path=/"; // different domain + std::string cookie04 = "cookieA=value; domain=example.com; path=/bar/"; // different path + std::string cookie05 = "cookieC; domain=example.com; path=/"; // empty value + std::string cookie06 = "cookieD=value; domain=example.com; path=/; expires="; // different name, persistent cookie + cookie06 += mFutureString; + + mCookieStore.setCookies(cookie01); + mCookieStore.setCookies(cookie02); + mCookieStore.setCookies(cookie03); + mCookieStore.setCookies(cookie04); + mCookieStore.setCookies(cookie05); + mCookieStore.setCookies(cookie06); + + // Invalid cookies (these will get parse errors and not be added to the store) + + std::string badcookie01 = "cookieD=value; domain=example.com; path=/; foo=bar"; // invalid field name + std::string badcookie02 = "cookieE=value; path=/"; // no domain + + mCookieStore.setCookies(badcookie01); + mCookieStore.setCookies(badcookie02); + + // All cookies added so far should have been marked as "changed" + setCookies(mCookieStore.getChangedCookies()); + ensureSize("count of changed cookies", 6); + ensureCookie(cookie01); + ensureCookie(cookie02); + ensureCookie(cookie03); + ensureCookie(cookie04); + ensureCookie(cookie05); + ensureCookie(cookie06); + + // Save off the current state of the cookie store (we'll restore it later) + std::string savedCookies = mCookieStore.getAllCookies(); + + // Test replacing cookies + std::string cookie01a = "cookieA=newvalue; domain=example.com; path=/"; // updated value + std::string cookie02a = "cookieB=newvalue; domain=example.com; path=/; expires="; // remove cookie (by setting an expire date in the past) + cookie02a += mPastString; + + mCookieStore.setCookies(cookie01a); + mCookieStore.setCookies(cookie02a); + + // test for getting changed cookies + setCookies(mCookieStore.getChangedCookies()); + ensureSize("count of updated cookies", 2); + ensureCookie(cookie01a); + ensureCookie(cookie02a); + + // and for the state of the store after getting changed cookies + setCookies(mCookieStore.getAllCookies()); + ensureSize("count of valid cookies", 5); + ensureCookie(cookie01a); + ensureCookie(cookie03); + ensureCookie(cookie04); + ensureCookie(cookie05); + ensureCookie(cookie06); + + // Check that only the persistent cookie is returned here + setCookies(mCookieStore.getPersistentCookies()); + ensureSize("count of persistent cookies", 1); + ensureCookie(cookie06); + + // Restore the cookie store to a previous state and verify + mCookieStore.setAllCookies(savedCookies); + + // Since setAllCookies defaults to not marking cookies as changed, this list should be empty. + setCookies(mCookieStore.getChangedCookies()); + ensureSize("count of changed cookies after restore", 0); + + // Verify that the restore worked as it should have. + setCookies(mCookieStore.getAllCookies()); + ensureSize("count of restored cookies", 6); + ensureCookie(cookie01); + ensureCookie(cookie02); + ensureCookie(cookie03); + ensureCookie(cookie04); + ensureCookie(cookie05); + ensureCookie(cookie06); + } + +} -- cgit v1.2.3 From 68870a1f59c11a173353698b994f4540b214d57f Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 31 Mar 2010 18:53:50 -0700 Subject: Added copyright header to new unit test. --- indra/llplugin/tests/llplugincookiestore_test.cpp | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'indra/llplugin') diff --git a/indra/llplugin/tests/llplugincookiestore_test.cpp b/indra/llplugin/tests/llplugincookiestore_test.cpp index e3e8ac9804..020d9c1977 100644 --- a/indra/llplugin/tests/llplugincookiestore_test.cpp +++ b/indra/llplugin/tests/llplugincookiestore_test.cpp @@ -1,3 +1,36 @@ +/** + * @file llplugincookiestore_test.cpp + * @brief Unit tests for LLPluginCookieStore. + * + * @cond + * $LicenseInfo:firstyear=2010&license=viewergpl$ + * + * Copyright (c) 2010, Linden Research, Inc. + * + * Second Life Viewer Source Code + * The source code in this file ("Source Code") is provided by Linden Lab + * to you under the terms of the GNU General Public License, version 2.0 + * ("GPL"), unless you have obtained a separate licensing agreement + * ("Other License"), formally executed by you and Linden Lab. Terms of + * the GPL can be found in doc/GPL-license.txt in this distribution, or + * online at http://secondlife.com/developers/opensource/gplv2 + * + * There are special exceptions to the terms and conditions of the GPL as + * it is applied to this Source Code. View the full text of the exception + * in the file doc/FLOSS-exception.txt in this software distribution, or + * online at http://secondlife.com/developers/opensource/flossexception + * + * By copying, modifying or distributing this software, you acknowledge + * that you have read and understood your obligations described above, + * and agree to abide by those obligations. + * + * ALL LINDEN LAB SOURCE CODE IS PROVIDED "AS IS." LINDEN LAB MAKES NO + * WARRANTIES, EXPRESS, IMPLIED OR OTHERWISE, REGARDING ITS ACCURACY, + * COMPLETENESS OR PERFORMANCE. + * $/LicenseInfo$ + * @endcond + */ + #include "linden_common.h" #include "../test/lltut.h" -- cgit v1.2.3 From 2ba90ca87174a6f29ae467b4677e4876cd113e8f Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 7 Apr 2010 18:15:56 -0700 Subject: Fix for EXT-6756: google apps auth doesn't work right with shared media cookies Added "HttpOnly" to the allowed field names in LLPluginCookieStore::Cookie::parse(). (This was the actual cause of the failure -- cookies with this field in them were silently failing to parse.) Added some LL_WARNS logging on this sort of cookie parse failure, which will make similar problems much easier to track down in future. Also added tags to most of the logging in llplugincookiestore.cpp to make it easier to selectively enable it when debugging. Added a cookie with all allowable field names to the unit test. Reviewed by Sam at http://codereview.lindenlab.com/1247014 --- indra/llplugin/llplugincookiestore.cpp | 36 +++++++++++++++-------- indra/llplugin/tests/llplugincookiestore_test.cpp | 2 +- 2 files changed, 24 insertions(+), 14 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llplugincookiestore.cpp b/indra/llplugin/llplugincookiestore.cpp index 85b1e70d78..da770c5f2e 100644 --- a/indra/llplugin/llplugincookiestore.cpp +++ b/indra/llplugin/llplugincookiestore.cpp @@ -99,7 +99,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) std::string::size_type cookie_end = mCookie.size(); std::string::size_type field_start = 0; - lldebugs << "parsing cookie: " << mCookie << llendl; + LL_DEBUGS("CookieStoreParse") << "parsing cookie: " << mCookie << LL_ENDL; while(field_start < cookie_end) { // Finding the start of the next field requires honoring special quoting rules @@ -167,10 +167,10 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) ++value_end; } - lldebugs + LL_DEBUGS("CookieStoreParse") << " field name: \"" << mCookie.substr(name_start, name_end - name_start) << "\", value: \"" << mCookie.substr(value_start, value_end - value_start) << "\"" - << llendl; + << LL_ENDL; // See whether this field is one we know if(first_field) @@ -195,13 +195,13 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) #if 1 time_t date = curl_getdate(date_string.c_str(), NULL ); mDate.secondsSinceEpoch((F64)date); - lldebugs << " expire date parsed to: " << mDate.asRFC1123() << llendl; + LL_DEBUGS("CookieStoreParse") << " expire date parsed to: " << mDate.asRFC1123() << LL_ENDL; #else // This doesn't work (rfc1123-format dates cause it to fail) if(!mDate.fromString(date_string)) { // Date failed to parse. - llwarns << "failed to parse cookie's expire date: " << date << llendl; + LL_WARNS("CookieStoreParse") << "failed to parse cookie's expire date: " << date << LL_ENDL; return false; } #endif @@ -232,9 +232,14 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) { // We don't care about the value of this field (yet) } + else if(matchName(name_start, name_end, "httponly")) + { + // We don't care about the value of this field (yet) + } else { // An unknown field is a parse failure + LL_WARNS("CookieStoreParse") << "unexpected field name: " << mCookie.substr(name_start, name_end - name_start) << LL_ENDL; return false; } @@ -270,7 +275,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) mCookie += host; mDomainEnd = mCookie.size(); - lldebugs << "added domain (" << mDomainStart << " to " << mDomainEnd << "), new cookie is: " << mCookie << llendl; + LL_DEBUGS("CookieStoreParse") << "added domain (" << mDomainStart << " to " << mDomainEnd << "), new cookie is: " << mCookie << LL_ENDL; } // If the cookie doesn't have a path, add "/". @@ -288,7 +293,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host) mCookie += "/"; mPathEnd = mCookie.size(); - lldebugs << "added path (" << mPathStart << " to " << mPathEnd << "), new cookie is: " << mCookie << llendl; + LL_DEBUGS("CookieStoreParse") << "added path (" << mPathStart << " to " << mPathEnd << "), new cookie is: " << mCookie << LL_ENDL; } @@ -566,7 +571,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t Cookie *cookie = Cookie::createFromString(s, cookie_start, cookie_end, host); if(cookie) { - lldebugs << "setting cookie: " << cookie->getCookie() << llendl; + LL_DEBUGS("CookieStoreUpdate") << "setting cookie: " << cookie->getCookie() << LL_ENDL; // Create a key for this cookie std::string key = cookie->getKey(); @@ -579,7 +584,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t { // If we're marking cookies as changed, we should keep it anyway since we'll need to send it out with deltas. cookie->setDead(true); - lldebugs << " marking dead" << llendl; + LL_DEBUGS("CookieStoreUpdate") << " marking dead" << LL_ENDL; } else { @@ -590,7 +595,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t delete cookie; cookie = NULL; - lldebugs << " removing" << llendl; + LL_DEBUGS("CookieStoreUpdate") << " removing" << LL_ENDL; } } @@ -607,7 +612,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t delete cookie; cookie = NULL; - lldebugs << " unchanged" << llendl; + LL_DEBUGS("CookieStoreUpdate") << " unchanged" << LL_ENDL; } else { @@ -619,7 +624,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t if(mark_changed) mHasChangedCookies = true; - lldebugs << " replacing" << llendl; + LL_DEBUGS("CookieStoreUpdate") << " replacing" << LL_ENDL; } } else @@ -631,10 +636,15 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t if(mark_changed) mHasChangedCookies = true; - lldebugs << " adding" << llendl; + LL_DEBUGS("CookieStoreUpdate") << " adding" << LL_ENDL; } } } + else + { + LL_WARNS("CookieStoreUpdate") << "failed to parse cookie: " << s.substr(cookie_start, cookie_end - cookie_start) << LL_ENDL; + } + } void LLPluginCookieStore::clearCookies() diff --git a/indra/llplugin/tests/llplugincookiestore_test.cpp b/indra/llplugin/tests/llplugincookiestore_test.cpp index 020d9c1977..c903464c64 100644 --- a/indra/llplugin/tests/llplugincookiestore_test.cpp +++ b/indra/llplugin/tests/llplugincookiestore_test.cpp @@ -127,7 +127,7 @@ namespace tut // Valid, distinct cookies: std::string cookie01 = "cookieA=value; domain=example.com; path=/"; - std::string cookie02 = "cookieB=value; domain=example.com; path=/"; // different name + std::string cookie02 = "cookieB=value; Domain=example.com; Path=/; Max-Age=10; Secure; Version=1; Comment=foo!; HTTPOnly"; // cookie with every supported field, in different cases. std::string cookie03 = "cookieA=value; domain=foo.example.com; path=/"; // different domain std::string cookie04 = "cookieA=value; domain=example.com; path=/bar/"; // different path std::string cookie05 = "cookieC; domain=example.com; path=/"; // empty value -- cgit v1.2.3 From 144b8349b3f0773ac575e178a0e3109d963be9a8 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Wed, 21 Apr 2010 17:04:27 -0700 Subject: Fix for EXT-6287 (fullscreen flash video plays behind the viewer instead of in front) Made SLPlugin into a bundled app on the Mac (this is apparently necessary for the plugin's window to be allowed to get focus). Changed the Info.plist key SLPlugin uses to keep itself out of the dock from LSBackgroundOnly to LSUIElement (this allows it to get focus and display UI). Added some Mac-specific code to slplugin.cpp to manipulate window layers and bring the plugin process to the foreground when something in the process opens a window, and to bring the viewer to the foreground when the last window in the process is closed. --- indra/llplugin/slplugin/CMakeLists.txt | 20 ++++-- indra/llplugin/slplugin/slplugin.cpp | 94 ++++++++++++++++++++++++++++- indra/llplugin/slplugin/slplugin_info.plist | 4 +- 3 files changed, 109 insertions(+), 9 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/slplugin/CMakeLists.txt b/indra/llplugin/slplugin/CMakeLists.txt index 4a7d670c23..c1536e85de 100644 --- a/indra/llplugin/slplugin/CMakeLists.txt +++ b/indra/llplugin/slplugin/CMakeLists.txt @@ -27,9 +27,15 @@ set(SLPlugin_SOURCE_FILES add_executable(SLPlugin WIN32 + MACOSX_BUNDLE ${SLPlugin_SOURCE_FILES} ) +set_target_properties(SLPlugin + PROPERTIES + MACOSX_BUNDLE_INFO_PLIST ${CMAKE_CURRENT_SOURCE_DIR}/slplugin_info.plist + ) + target_link_libraries(SLPlugin ${LLPLUGIN_LIBRARIES} ${LLMESSAGE_LIBRARIES} @@ -44,12 +50,16 @@ add_dependencies(SLPlugin ) if (DARWIN) - # Mac version needs to link against carbon, and also needs an embedded plist (to set LSBackgroundOnly) + # Mac version needs to link against Carbon target_link_libraries(SLPlugin ${CARBON_LIBRARY}) - set_target_properties( - SLPlugin - PROPERTIES - LINK_FLAGS "-Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_SOURCE_DIR}/slplugin_info.plist" + # Make sure the app bundle has a Resources directory (it will get populated by viewer-manifest.py later) + add_custom_command( + TARGET SLPlugin POST_BUILD + COMMAND mkdir + ARGS + -p + ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/SLPlugin.app/Contents/Resources ) endif (DARWIN) +ll_deploy_sharedlibs_command(SLPlugin) diff --git a/indra/llplugin/slplugin/slplugin.cpp b/indra/llplugin/slplugin/slplugin.cpp index 77240ce546..c18e2375f9 100644 --- a/indra/llplugin/slplugin/slplugin.cpp +++ b/indra/llplugin/slplugin/slplugin.cpp @@ -51,7 +51,7 @@ #endif /* - On Mac OS, since we call WaitNextEvent, this process will show up in the dock unless we set the LSBackgroundOnly flag in the Info.plist. + On Mac OS, since we call WaitNextEvent, this process will show up in the dock unless we set the LSBackgroundOnly or LSUIElement flag in the Info.plist. Normally non-bundled binaries don't have an info.plist file, but it's possible to embed one in the binary by adding this to the linker flags: @@ -60,7 +60,8 @@ which means adding this to the gcc flags: -Wl,-sectcreate,__TEXT,__info_plist,/path/to/slplugin_info.plist - + + Now that SLPlugin is a bundled app on the Mac, this is no longer necessary (it can just use a regular Info.plist file), but I'm leaving this comment in for posterity. */ #if LL_DARWIN || LL_LINUX @@ -239,6 +240,21 @@ int main(int argc, char **argv) checkExceptionHandler(); #endif +#if LL_DARWIN + // If the plugin opens a new window (such as the Flash plugin's fullscreen player), we may need to bring this plugin process to the foreground. + // Use this to track the current frontmost window and bring this process to the front if it changes. + WindowRef front_window = NULL; + WindowGroupRef layer_group = NULL; + int window_hack_state = 0; + CreateWindowGroup(kWindowGroupAttrFixedLevel, &layer_group); + if(layer_group) + { + // Start out with a window layer that's way out in front (fixes the problem with the menubar not getting hidden on first switch to fullscreen youtube) + SetWindowGroupName(layer_group, CFSTR("SLPlugin Layer")); + SetWindowGroupLevel(layer_group, kCGOverlayWindowLevel); + } +#endif + while(!plugin->isDone()) { timer.reset(); @@ -248,6 +264,80 @@ int main(int argc, char **argv) // Some plugins (webkit at least) will want an event loop. This qualifies. EventRecord evt; WaitNextEvent(0, &evt, 0, NULL); + + // Check for a change in this process's frontmost window. + if(FrontWindow() != front_window) + { + ProcessSerialNumber self = { 0, kCurrentProcess }; + ProcessSerialNumber parent = { 0, kNoProcess }; + ProcessSerialNumber front = { 0, kNoProcess }; + Boolean this_is_front_process = false; + Boolean parent_is_front_process = false; + { + // Get this process's parent + ProcessInfoRec info; + info.processInfoLength = sizeof(ProcessInfoRec); + info.processName = NULL; + info.processAppSpec = NULL; + if(GetProcessInformation( &self, &info ) == noErr) + { + parent = info.processLauncher; + } + + // and figure out whether this process or its parent are currently frontmost + if(GetFrontProcess(&front) == noErr) + { + (void) SameProcess(&self, &front, &this_is_front_process); + (void) SameProcess(&parent, &front, &parent_is_front_process); + } + } + + if((FrontWindow() != NULL) && (front_window == NULL)) + { + // Opening the first window + + if(window_hack_state == 0) + { + // Next time through the event loop, lower the window group layer + window_hack_state = 1; + } + + if(layer_group) + { + SetWindowGroup(FrontWindow(), layer_group); + } + + if(parent_is_front_process) + { + // Bring this process's windows to the front. + (void) SetFrontProcess( &self ); + } + + ActivateWindow(FrontWindow(), true); + } + else if((FrontWindow() == NULL) && (front_window != NULL)) + { + // Closing the last window + + if(this_is_front_process) + { + // Try to bring this process's parent to the front + (void) SetFrontProcess(&parent); + } + } + else if(window_hack_state == 1) + { + if(layer_group) + { + // Set the window group level back to something less extreme + SetWindowGroupLevel(layer_group, kCGNormalWindowLevel); + } + window_hack_state = 2; + } + + front_window = FrontWindow(); + + } } #endif F64 elapsed = timer.getElapsedTimeF64(); diff --git a/indra/llplugin/slplugin/slplugin_info.plist b/indra/llplugin/slplugin/slplugin_info.plist index b1daf87424..c4597380e0 100644 --- a/indra/llplugin/slplugin/slplugin_info.plist +++ b/indra/llplugin/slplugin/slplugin_info.plist @@ -6,7 +6,7 @@ English CFBundleInfoDictionaryVersion 6.0 - LSBackgroundOnly - + LSUIElement + 1 -- cgit v1.2.3 From 0cdaca2b468bbc0f6d9e7f88bd7e39f8c7c04707 Mon Sep 17 00:00:00 2001 From: Aimee Linden Date: Sun, 25 Apr 2010 03:49:55 +0100 Subject: DEV-49389 (SNOW-571 / VWR-18279): Replacing another deprecated WaitNextEvent() for the OS X 10.6 SDK Replaced the deprecated WaitNextEvent() call with ReceiveNextEvent() and SendEventToEventTarget() as a basic event loop in slplugin.cpp --- indra/llplugin/slplugin/slplugin.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/slplugin/slplugin.cpp b/indra/llplugin/slplugin/slplugin.cpp index c18e2375f9..7d6dde1a58 100644 --- a/indra/llplugin/slplugin/slplugin.cpp +++ b/indra/llplugin/slplugin/slplugin.cpp @@ -255,6 +255,9 @@ int main(int argc, char **argv) } #endif +#if LL_DARWIN + EventTargetRef event_target = GetEventDispatcherTarget(); +#endif while(!plugin->isDone()) { timer.reset(); @@ -262,8 +265,12 @@ int main(int argc, char **argv) #if LL_DARWIN { // Some plugins (webkit at least) will want an event loop. This qualifies. - EventRecord evt; - WaitNextEvent(0, &evt, 0, NULL); + EventRef event; + if(ReceiveNextEvent(0, 0, kEventDurationNoWait, true, &event) == noErr) + { + SendEventToEventTarget (event, event_target); + ReleaseEvent(event); + } // Check for a change in this process's frontmost window. if(FrontWindow() != front_window) -- cgit v1.2.3 From 57a2a2beff565c9a2e2ed0937df178c38d114cb3 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Fri, 23 Apr 2010 15:50:22 -0700 Subject: Add a way for plugins to send a message and block waiting for the response This requires some cooperation between the plugin and the host, and will only work for specific messages. The way it works is as follows: * the plugin sends a message containing the key "blocking_request" (with any value) * this will cause the "send message" function to block (continuing to pull incoming messages off the network socket and queue them) until it receives a message from the host containing the key "blocking_response" ** NOTE: if the plugin sends a blocking_request that isn't set up to cause the host to send back a blocking_response, it will block forever * the blocking_response message will be handed to the plugin's "receive message" function _immediately_ (before the "send message" function returns) ** this means that the plugin can extract response data for use by the the code that sent the message (but is still blocked inside the "send message" call) ** NOTE: this BREAKS the invariant stating that the plugin's "receive message" function will never be called recursively, and the plugin MUST be prepared to deal with this * after the plugin finishes processing the blocking_response message, the "send message" function that was called with the blocking_request message will return to the plugin * any queued messages will be delivered after the outer invocation of the plugin's "receive message" function returns (as normal) Inside the viewer, the code can tell when a plugin is in this blocked state by calling LLPluginProcessParent::isBlocked(). LLPluginClassMedia uses this to avoid sending mouse-move and size-change messages to blocked plugins. --- indra/llplugin/llpluginclassmedia.cpp | 7 +++- indra/llplugin/llpluginmessagepipe.cpp | 17 ++++---- indra/llplugin/llpluginprocesschild.cpp | 68 +++++++++++++++++++++++++++++--- indra/llplugin/llpluginprocesschild.h | 5 +++ indra/llplugin/llpluginprocessparent.cpp | 38 ++++++++++++------ indra/llplugin/llpluginprocessparent.h | 4 ++ 6 files changed, 112 insertions(+), 27 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index e09b511a6e..24c671437e 100644 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -160,7 +160,7 @@ void LLPluginClassMedia::idle(void) mPlugin->idle(); } - if((mMediaWidth == -1) || (!mTextureParamsReceived) || (mPlugin == NULL)) + if((mMediaWidth == -1) || (!mTextureParamsReceived) || (mPlugin == NULL) || (mPlugin->isBlocked())) { // Can't process a size change at this time } @@ -437,6 +437,11 @@ void LLPluginClassMedia::mouseEvent(EMouseEventType type, int button, int x, int { if(type == MOUSE_EVENT_MOVE) { + if(!mPlugin || !mPlugin->isRunning() || mPlugin->isBlocked()) + { + // Don't queue up mouse move events that can't be delivered. + } + if((x == mLastMouseX) && (y == mLastMouseY)) { // Don't spam unnecessary mouse move events. diff --git a/indra/llplugin/llpluginmessagepipe.cpp b/indra/llplugin/llpluginmessagepipe.cpp index 1d7ddc5592..e524c88cf8 100644 --- a/indra/llplugin/llpluginmessagepipe.cpp +++ b/indra/llplugin/llpluginmessagepipe.cpp @@ -299,26 +299,23 @@ bool LLPluginMessagePipe::pump(F64 timeout) void LLPluginMessagePipe::processInput(void) { // Look for input delimiter(s) in the input buffer. - int start = 0; int delim; - while((delim = mInput.find(MESSAGE_DELIMITER, start)) != std::string::npos) + while((delim = mInput.find(MESSAGE_DELIMITER)) != std::string::npos) { // Let the owner process this message if (mOwner) { - mOwner->receiveMessageRaw(mInput.substr(start, delim - start)); + // Pull the message out of the input buffer before calling receiveMessageRaw. + // It's now possible for this function to get called recursively (in the case where the plugin makes a blocking request) + // and this guarantees that the messages will get dequeued correctly. + std::string message(mInput, 0, delim); + mInput.erase(0, delim + 1); + mOwner->receiveMessageRaw(message); } else { LL_WARNS("Plugin") << "!mOwner" << LL_ENDL; } - - start = delim + 1; } - - // Remove delivered messages from the input buffer. - if(start != 0) - mInput = mInput.substr(start); - } diff --git a/indra/llplugin/llpluginprocesschild.cpp b/indra/llplugin/llpluginprocesschild.cpp index ccaf95b36d..2d078cd6ed 100644 --- a/indra/llplugin/llpluginprocesschild.cpp +++ b/indra/llplugin/llpluginprocesschild.cpp @@ -48,6 +48,8 @@ LLPluginProcessChild::LLPluginProcessChild() mSocket = LLSocket::create(gAPRPoolp, LLSocket::STREAM_TCP); mSleepTime = PLUGIN_IDLE_SECONDS; // default: send idle messages at 100Hz mCPUElapsed = 0.0f; + mBlockingRequest = false; + mBlockingResponseReceived = false; } LLPluginProcessChild::~LLPluginProcessChild() @@ -226,6 +228,7 @@ void LLPluginProcessChild::idle(void) void LLPluginProcessChild::sleep(F64 seconds) { + deliverQueuedMessages(); if(mMessagePipe) { mMessagePipe->pump(seconds); @@ -238,6 +241,7 @@ void LLPluginProcessChild::sleep(F64 seconds) void LLPluginProcessChild::pump(void) { + deliverQueuedMessages(); if(mMessagePipe) { mMessagePipe->pump(0.0f); @@ -309,15 +313,32 @@ void LLPluginProcessChild::receiveMessageRaw(const std::string &message) LL_DEBUGS("Plugin") << "Received from parent: " << message << LL_ENDL; + // Decode this message + LLPluginMessage parsed; + parsed.parse(message); + + if(mBlockingRequest) + { + // We're blocking the plugin waiting for a response. + + if(parsed.hasValue("blocking_response")) + { + // This is the message we've been waiting for -- fall through and send it immediately. + mBlockingResponseReceived = true; + } + else + { + // Still waiting. Queue this message and don't process it yet. + mMessageQueue.push(message); + return; + } + } + bool passMessage = true; // FIXME: how should we handle queueing here? { - // Decode this message - LLPluginMessage parsed; - parsed.parse(message); - std::string message_class = parsed.getClass(); if(message_class == LLPLUGIN_MESSAGE_CLASS_INTERNAL) { @@ -425,7 +446,13 @@ void LLPluginProcessChild::receiveMessageRaw(const std::string &message) void LLPluginProcessChild::receivePluginMessage(const std::string &message) { LL_DEBUGS("Plugin") << "Received from plugin: " << message << LL_ENDL; - + + if(mBlockingRequest) + { + // + LL_ERRS("Plugin") << "Can't send a message while already waiting on a blocking request -- aborting!" << LL_ENDL; + } + // Incoming message from the plugin instance bool passMessage = true; @@ -436,6 +463,12 @@ void LLPluginProcessChild::receivePluginMessage(const std::string &message) // Decode this message LLPluginMessage parsed; parsed.parse(message); + + if(parsed.hasValue("blocking_request")) + { + mBlockingRequest = true; + } + std::string message_class = parsed.getClass(); if(message_class == "base") { @@ -494,6 +527,19 @@ void LLPluginProcessChild::receivePluginMessage(const std::string &message) LL_DEBUGS("Plugin") << "Passing through to parent: " << message << LL_ENDL; writeMessageRaw(message); } + + while(mBlockingRequest) + { + // The plugin wants to block and wait for a response to this message. + sleep(mSleepTime); // this will pump the message pipe and process messages + + if(mBlockingResponseReceived || mSocketError != APR_SUCCESS || (mMessagePipe == NULL)) + { + // Response has been received, or we've hit an error state. Stop waiting. + mBlockingRequest = false; + mBlockingResponseReceived = false; + } + } } @@ -502,3 +548,15 @@ void LLPluginProcessChild::setState(EState state) LL_DEBUGS("Plugin") << "setting state to " << state << LL_ENDL; mState = state; }; + +void LLPluginProcessChild::deliverQueuedMessages() +{ + if(!mBlockingRequest) + { + while(!mMessageQueue.empty()) + { + receiveMessageRaw(mMessageQueue.front()); + mMessageQueue.pop(); + } + } +} diff --git a/indra/llplugin/llpluginprocesschild.h b/indra/llplugin/llpluginprocesschild.h index 0e5e85406a..1430ad7a5d 100644 --- a/indra/llplugin/llpluginprocesschild.h +++ b/indra/llplugin/llpluginprocesschild.h @@ -106,6 +106,11 @@ private: LLTimer mHeartbeat; F64 mSleepTime; F64 mCPUElapsed; + bool mBlockingRequest; + bool mBlockingResponseReceived; + std::queue mMessageQueue; + + void deliverQueuedMessages(); }; diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index 895c858979..e273410a1d 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -54,6 +54,7 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner) mCPUUsage = 0.0; mDisableTimeout = false; mDebug = false; + mBlocked = false; mPluginLaunchTimeout = 60.0f; mPluginLockupTimeout = 15.0f; @@ -479,6 +480,13 @@ void LLPluginProcessParent::setSleepTime(F64 sleep_time, bool force_send) void LLPluginProcessParent::sendMessage(const LLPluginMessage &message) { + if(message.hasValue("blocking_response")) + { + mBlocked = false; + + // reset the heartbeat timer, since there will have been no heartbeats while the plugin was blocked. + mHeartbeat.setTimerExpirySec(mPluginLockupTimeout); + } std::string buffer = message.generate(); LL_DEBUGS("Plugin") << "Sending: " << buffer << LL_ENDL; @@ -501,6 +509,11 @@ void LLPluginProcessParent::receiveMessageRaw(const std::string &message) void LLPluginProcessParent::receiveMessage(const LLPluginMessage &message) { + if(message.hasValue("blocking_request")) + { + mBlocked = true; + } + std::string message_class = message.getClass(); if(message_class == LLPLUGIN_MESSAGE_CLASS_INTERNAL) { @@ -689,18 +702,15 @@ bool LLPluginProcessParent::pluginLockedUpOrQuit() { bool result = false; - if(!mDisableTimeout && !mDebug) + if(!mProcess.isRunning()) { - if(!mProcess.isRunning()) - { - LL_WARNS("Plugin") << "child exited" << llendl; - result = true; - } - else if(pluginLockedUp()) - { - LL_WARNS("Plugin") << "timeout" << llendl; - result = true; - } + LL_WARNS("Plugin") << "child exited" << llendl; + result = true; + } + else if(pluginLockedUp()) + { + LL_WARNS("Plugin") << "timeout" << llendl; + result = true; } return result; @@ -708,6 +718,12 @@ bool LLPluginProcessParent::pluginLockedUpOrQuit() bool LLPluginProcessParent::pluginLockedUp() { + if(mDisableTimeout || mDebug || mBlocked) + { + // Never time out a plugin process in these cases. + return false; + } + // If the timer is running and has expired, the plugin has locked up. return (mHeartbeat.getStarted() && mHeartbeat.hasExpired()); } diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index cc6c513615..31f125bfb3 100644 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -74,6 +74,9 @@ public: // returns true if the process has exited or we've had a fatal error bool isDone(void); + // returns true if the process is currently waiting on a blocking request + bool isBlocked(void) { return mBlocked; }; + void killSockets(void); // Go to the proper error state @@ -160,6 +163,7 @@ private: bool mDisableTimeout; bool mDebug; + bool mBlocked; LLProcessLauncher mDebugger; -- cgit v1.2.3 From dacc5afbf0f1bde7454c1eadf56edb669d0741a9 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Tue, 27 Apr 2010 17:25:01 -0700 Subject: Architectural changes to LLPlugin message processing. LLPluginProcessParent can now optionally use a separate thread for reading messages from plugin sockets. If this is enabled, it will spawn a single thread and use apr_pollset_poll to wake up the thread when incoming data arrives instead of polling all the descriptors round-robin every frame. This should be somewhat more efficient, and should also allow blocking requests from plugins to be serviced much more quickly (once we start using them). This is currently disabled by default, until it's had a bit more focused testing on multiple platforms. Hooked up the switch to use the message read thread to the PluginUseReadThread debug setting and an item in the Advanced menu in the viewer, and to a checkbox in the UI in llmediaplugintest. Updated some debug logging in the plugin system to have appropriate tags and not log dire-looking warnings during normal operation. LLPluginProcessParent now once again explicitly kills plugin processes (instead of just closing their sockets and waiting for them to exit). The problem we were attempting to solve by not doing the kill (letting the webkit plugin write its cookie file on exit) has been solved another way. LLPluginProcessParent::sendMessage() now attempts to write the outgoing message to the socket immediately instead of waiting for the next frame. This should reduce the latency of sending plugin messages. Added a separate fast timer for LLViewerMedia::updateMedia(). --- indra/llplugin/llpluginmessagepipe.cpp | 107 ++++++-- indra/llplugin/llpluginmessagepipe.h | 9 +- indra/llplugin/llpluginprocesschild.cpp | 9 +- indra/llplugin/llpluginprocessparent.cpp | 414 +++++++++++++++++++++++++++++-- indra/llplugin/llpluginprocessparent.h | 26 +- 5 files changed, 515 insertions(+), 50 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginmessagepipe.cpp b/indra/llplugin/llpluginmessagepipe.cpp index e524c88cf8..a62534de95 100644 --- a/indra/llplugin/llpluginmessagepipe.cpp +++ b/indra/llplugin/llpluginmessagepipe.cpp @@ -96,11 +96,14 @@ void LLPluginMessagePipeOwner::killMessagePipe(void) } } -LLPluginMessagePipe::LLPluginMessagePipe(LLPluginMessagePipeOwner *owner, LLSocket::ptr_t socket) +LLPluginMessagePipe::LLPluginMessagePipe(LLPluginMessagePipeOwner *owner, LLSocket::ptr_t socket): + mInputMutex(gAPRPoolp), + mOutputMutex(gAPRPoolp), + mOwner(owner), + mSocket(socket) { - mOwner = owner; + mOwner->setMessagePipe(this); - mSocket = socket; } LLPluginMessagePipe::~LLPluginMessagePipe() @@ -114,8 +117,10 @@ LLPluginMessagePipe::~LLPluginMessagePipe() bool LLPluginMessagePipe::addMessage(const std::string &message) { // queue the message for later output + mOutputMutex.lock(); mOutput += message; mOutput += MESSAGE_DELIMITER; // message separator + mOutputMutex.unlock(); return true; } @@ -148,6 +153,18 @@ void LLPluginMessagePipe::setSocketTimeout(apr_interval_time_t timeout_usec) } bool LLPluginMessagePipe::pump(F64 timeout) +{ + bool result = pumpOutput(); + + if(result) + { + result = pumpInput(timeout); + } + + return result; +} + +bool LLPluginMessagePipe::pumpOutput() { bool result = true; @@ -156,6 +173,7 @@ bool LLPluginMessagePipe::pump(F64 timeout) apr_status_t status; apr_size_t size; + mOutputMutex.lock(); if(!mOutput.empty()) { // write any outgoing messages @@ -183,6 +201,17 @@ bool LLPluginMessagePipe::pump(F64 timeout) // remove the written part from the buffer and try again later. mOutput = mOutput.substr(size); } + else if(APR_STATUS_IS_EOF(status)) + { + // This is what we normally expect when a plugin exits. + llinfos << "Got EOF from plugin socket. " << llendl; + + if(mOwner) + { + mOwner->socketError(status); + } + result = false; + } else { // some other error @@ -196,6 +225,20 @@ bool LLPluginMessagePipe::pump(F64 timeout) result = false; } } + mOutputMutex.unlock(); + } + + return result; +} + +bool LLPluginMessagePipe::pumpInput(F64 timeout) +{ + bool result = true; + + if(mSocket) + { + apr_status_t status; + apr_size_t size; // FIXME: For some reason, the apr timeout stuff isn't working properly on windows. // Until such time as we figure out why, don't try to use the socket timeout -- just sleep here instead. @@ -216,8 +259,16 @@ bool LLPluginMessagePipe::pump(F64 timeout) char input_buf[1024]; apr_size_t request_size; - // Start out by reading one byte, so that any data received will wake us up. - request_size = 1; + if(timeout == 0.0f) + { + // If we have no timeout, start out with a full read. + request_size = sizeof(input_buf); + } + else + { + // Start out by reading one byte, so that any data received will wake us up. + request_size = 1; + } // and use the timeout so we'll sleep if no data is available. setSocketTimeout((apr_interval_time_t)(timeout * 1000000)); @@ -236,11 +287,15 @@ bool LLPluginMessagePipe::pump(F64 timeout) // LL_INFOS("Plugin") << "after apr_socket_recv, size = " << size << LL_ENDL; if(size > 0) + { + mInputMutex.lock(); mInput.append(input_buf, size); + mInputMutex.unlock(); + } if(status == APR_SUCCESS) { -// llinfos << "success, read " << size << llendl; + LL_DEBUGS("PluginSocket") << "success, read " << size << LL_ENDL; if(size != request_size) { @@ -250,16 +305,28 @@ bool LLPluginMessagePipe::pump(F64 timeout) } else if(APR_STATUS_IS_TIMEUP(status)) { -// llinfos << "TIMEUP, read " << size << llendl; + LL_DEBUGS("PluginSocket") << "TIMEUP, read " << size << LL_ENDL; // Timeout was hit. Since the initial read is 1 byte, this should never be a partial read. break; } else if(APR_STATUS_IS_EAGAIN(status)) { -// llinfos << "EAGAIN, read " << size << llendl; + LL_DEBUGS("PluginSocket") << "EAGAIN, read " << size << LL_ENDL; - // We've been doing partial reads, and we're done now. + // Non-blocking read returned immediately. + break; + } + else if(APR_STATUS_IS_EOF(status)) + { + // This is what we normally expect when a plugin exits. + LL_INFOS("PluginSocket") << "Got EOF from plugin socket. " << LL_ENDL; + + if(mOwner) + { + mOwner->socketError(status); + } + result = false; break; } else @@ -276,22 +343,18 @@ bool LLPluginMessagePipe::pump(F64 timeout) break; } - // Second and subsequent reads should not use the timeout - setSocketTimeout(0); - // and should try to fill the input buffer - request_size = sizeof(input_buf); + if(timeout != 0.0f) + { + // Second and subsequent reads should not use the timeout + setSocketTimeout(0); + // and should try to fill the input buffer + request_size = sizeof(input_buf); + } } processInput(); } } - - if(!result) - { - // If we got an error, we're done. - LL_INFOS("Plugin") << "Error from socket, cleaning up." << LL_ENDL; - delete this; - } return result; } @@ -300,6 +363,7 @@ void LLPluginMessagePipe::processInput(void) { // Look for input delimiter(s) in the input buffer. int delim; + mInputMutex.lock(); while((delim = mInput.find(MESSAGE_DELIMITER)) != std::string::npos) { // Let the owner process this message @@ -310,12 +374,15 @@ void LLPluginMessagePipe::processInput(void) // and this guarantees that the messages will get dequeued correctly. std::string message(mInput, 0, delim); mInput.erase(0, delim + 1); + mInputMutex.unlock(); mOwner->receiveMessageRaw(message); + mInputMutex.lock(); } else { LL_WARNS("Plugin") << "!mOwner" << LL_ENDL; } } + mInputMutex.unlock(); } diff --git a/indra/llplugin/llpluginmessagepipe.h b/indra/llplugin/llpluginmessagepipe.h index 1ddb38de68..1b0a08254b 100644 --- a/indra/llplugin/llpluginmessagepipe.h +++ b/indra/llplugin/llpluginmessagepipe.h @@ -35,6 +35,7 @@ #define LL_LLPLUGINMESSAGEPIPE_H #include "lliosocket.h" +#include "llthread.h" class LLPluginMessagePipe; @@ -51,7 +52,7 @@ public: virtual apr_status_t socketError(apr_status_t error); // called from LLPluginMessagePipe to manage the connection with LLPluginMessagePipeOwner -- do not use! - virtual void setMessagePipe(LLPluginMessagePipe *message_pipe) ; + virtual void setMessagePipe(LLPluginMessagePipe *message_pipe); protected: // returns false if writeMessageRaw() would drop the message @@ -76,14 +77,18 @@ public: void clearOwner(void); bool pump(F64 timeout = 0.0f); - + bool pumpOutput(); + bool pumpInput(F64 timeout = 0.0f); + protected: void processInput(void); // used internally by pump() void setSocketTimeout(apr_interval_time_t timeout_usec); + LLMutex mInputMutex; std::string mInput; + LLMutex mOutputMutex; std::string mOutput; LLPluginMessagePipeOwner *mOwner; diff --git a/indra/llplugin/llpluginprocesschild.cpp b/indra/llplugin/llpluginprocesschild.cpp index 2d078cd6ed..d1cf91b253 100644 --- a/indra/llplugin/llpluginprocesschild.cpp +++ b/indra/llplugin/llpluginprocesschild.cpp @@ -85,9 +85,14 @@ void LLPluginProcessChild::idle(void) bool idle_again; do { - if(mSocketError != APR_SUCCESS) + if(APR_STATUS_IS_EOF(mSocketError)) { - LL_INFOS("Plugin") << "message pipe is in error state, moving to STATE_ERROR"<< LL_ENDL; + // Plugin socket was closed. This covers both normal plugin termination and host crashes. + setState(STATE_ERROR); + } + else if(mSocketError != APR_SUCCESS) + { + LL_INFOS("Plugin") << "message pipe is in error state (" << mSocketError << "), moving to STATE_ERROR"<< LL_ENDL; setState(STATE_ERROR); } diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index e273410a1d..c61a903a2c 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -45,8 +45,51 @@ LLPluginProcessParentOwner::~LLPluginProcessParentOwner() } -LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner) +bool LLPluginProcessParent::sUseReadThread = false; +apr_pollset_t *LLPluginProcessParent::sPollSet = NULL; +bool LLPluginProcessParent::sPollsetNeedsRebuild = false; +LLMutex *LLPluginProcessParent::sInstancesMutex; +std::list LLPluginProcessParent::sInstances; +LLThread *LLPluginProcessParent::sPollThread = NULL; + + +class LLPluginProcessParentPollThread: public LLThread +{ +public: + LLPluginProcessParentPollThread() : + LLThread("LLPluginProcessParentPollThread", gAPRPoolp) + { + } +protected: + // Inherited from LLThread + /*virtual*/ void run(void) + { + while(!isQuitting() && LLPluginProcessParent::getUseReadThread()) + { + LLPluginProcessParent::poll(0.1f); + checkPause(); + } + + // Final poll to clean up the pollset, etc. + LLPluginProcessParent::poll(0.0f); + } + + // Inherited from LLThread + /*virtual*/ bool runCondition(void) + { + return(LLPluginProcessParent::canPollThreadRun()); + } + +}; + +LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): + mIncomingQueueMutex(gAPRPoolp) { + if(!sInstancesMutex) + { + sInstancesMutex = new LLMutex(gAPRPoolp); + } + mOwner = owner; mBoundPort = 0; mState = STATE_UNINITIALIZED; @@ -55,18 +98,30 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner) mDisableTimeout = false; mDebug = false; mBlocked = false; + mPolledInput = false; + mPollFD.client_data = NULL; mPluginLaunchTimeout = 60.0f; mPluginLockupTimeout = 15.0f; // Don't start the timer here -- start it when we actually launch the plugin process. mHeartbeat.stop(); + + // Don't add to the global list until fully constructed. + sInstancesMutex->lock(); + sInstances.push_back(this); + sInstancesMutex->unlock(); } LLPluginProcessParent::~LLPluginProcessParent() { LL_DEBUGS("Plugin") << "destructor" << LL_ENDL; + // Remove from the global list before beginning destruction. + sInstancesMutex->lock(); + sInstances.remove(this); + sInstancesMutex->unlock(); + // Destroy any remaining shared memory regions sharedMemoryRegionsType::iterator iter; while((iter = mSharedMemoryRegions.begin()) != mSharedMemoryRegions.end()) @@ -78,15 +133,15 @@ LLPluginProcessParent::~LLPluginProcessParent() mSharedMemoryRegions.erase(iter); } - // orphaning the process means it won't be killed when the LLProcessLauncher is destructed. - // This is what we want -- it should exit cleanly once it notices the sockets have been closed. - mProcess.orphan(); + mProcess.kill(); killSockets(); } void LLPluginProcessParent::killSockets(void) { + mIncomingQueueMutex.lock(); killMessagePipe(); + mIncomingQueueMutex.unlock(); mListenSocket.reset(); mSocket.reset(); } @@ -160,21 +215,47 @@ void LLPluginProcessParent::idle(void) do { + // process queued messages + mIncomingQueueMutex.lock(); + while(!mIncomingQueue.empty()) + { + LLPluginMessage message = mIncomingQueue.front(); + mIncomingQueue.pop(); + mIncomingQueueMutex.unlock(); + + receiveMessage(message); + + mIncomingQueueMutex.lock(); + } + + mIncomingQueueMutex.unlock(); + // Give time to network processing if(mMessagePipe) { - if(!mMessagePipe->pump()) + // Drain any queued outgoing messages + mMessagePipe->pumpOutput(); + + // Only do input processing here if this instance isn't in a pollset. + if(!mPolledInput) { -// LL_WARNS("Plugin") << "Message pipe hit an error state" << LL_ENDL; - errorState(); + mMessagePipe->pumpInput(); } } - - if((mSocketError != APR_SUCCESS) && (mState <= STATE_RUNNING)) + + if(mState <= STATE_RUNNING) { - // The socket is in an error state -- the plugin is gone. - LL_WARNS("Plugin") << "Socket hit an error state (" << mSocketError << ")" << LL_ENDL; - errorState(); + if(APR_STATUS_IS_EOF(mSocketError)) + { + // Plugin socket was closed. This covers both normal plugin termination and plugin crashes. + errorState(); + } + else if(mSocketError != APR_SUCCESS) + { + // The socket is in an error state -- the plugin is gone. + LL_WARNS("Plugin") << "Socket hit an error state (" << mSocketError << ")" << LL_ENDL; + errorState(); + } } // If a state needs to go directly to another state (as a performance enhancement), it can set idle_again to true after calling setState(). @@ -355,7 +436,7 @@ void LLPluginProcessParent::idle(void) break; case STATE_HELLO: - LL_DEBUGS("Plugin") << "received hello message" << llendl; + LL_DEBUGS("Plugin") << "received hello message" << LL_ENDL; // Send the message to load the plugin { @@ -389,7 +470,7 @@ void LLPluginProcessParent::idle(void) } else if(pluginLockedUp()) { - LL_WARNS("Plugin") << "timeout in exiting state, bailing out" << llendl; + LL_WARNS("Plugin") << "timeout in exiting state, bailing out" << LL_ENDL; errorState(); } break; @@ -411,8 +492,7 @@ void LLPluginProcessParent::idle(void) break; case STATE_CLEANUP: - // Don't do a kill here anymore -- closing the sockets is the new 'kill'. - mProcess.orphan(); + mProcess.kill(); killSockets(); setState(STATE_DONE); break; @@ -491,29 +571,315 @@ void LLPluginProcessParent::sendMessage(const LLPluginMessage &message) std::string buffer = message.generate(); LL_DEBUGS("Plugin") << "Sending: " << buffer << LL_ENDL; writeMessageRaw(buffer); + + // Try to send message immediately. + if(mMessagePipe) + { + mMessagePipe->pumpOutput(); + } +} + +//virtual +void LLPluginProcessParent::setMessagePipe(LLPluginMessagePipe *message_pipe) +{ + bool update_pollset = false; + + if(mMessagePipe) + { + // Unsetting an existing message pipe -- remove from the pollset + mPollFD.client_data = NULL; + + // pollset needs an update + update_pollset = true; + } + if(message_pipe != NULL) + { + // Set up the apr_pollfd_t + mPollFD.p = gAPRPoolp; + mPollFD.desc_type = APR_POLL_SOCKET; + mPollFD.reqevents = APR_POLLIN|APR_POLLERR|APR_POLLHUP; + mPollFD.rtnevents = 0; + mPollFD.desc.s = mSocket->getSocket(); + mPollFD.client_data = (void*)this; + + // pollset needs an update + update_pollset = true; + } + + mMessagePipe = message_pipe; + + if(update_pollset) + { + dirtyPollSet(); + } } +//static +void LLPluginProcessParent::dirtyPollSet() +{ + sPollsetNeedsRebuild = true; + + if(sPollThread) + { + LL_DEBUGS("PluginPoll") << "unpausing polling thread " << LL_ENDL; + sPollThread->unpause(); + } +} + +void LLPluginProcessParent::updatePollset() +{ + if(!sInstancesMutex) + { + // No instances have been created yet. There's no work to do. + return; + } + + sInstancesMutex->lock(); + + if(sPollSet) + { + LL_DEBUGS("PluginPoll") << "destroying pollset " << sPollSet << LL_ENDL; + // delete the existing pollset. + apr_pollset_destroy(sPollSet); + sPollSet = NULL; + } + + std::list::iterator iter; + int count = 0; + + // Count the number of instances that want to be in the pollset + for(iter = sInstances.begin(); iter != sInstances.end(); iter++) + { + (*iter)->mPolledInput = false; + if((*iter)->mPollFD.client_data) + { + // This instance has a socket that needs to be polled. + ++count; + } + } + + if(sUseReadThread && sPollThread && !sPollThread->isQuitting()) + { + if(!sPollSet && (count > 0)) + { + // The pollset doesn't exist yet. Create it now. + apr_status_t status = apr_pollset_create(&sPollSet, count, gAPRPoolp, APR_POLLSET_NOCOPY); + if(status != APR_SUCCESS) + { + LL_WARNS("PluginPoll") << "Couldn't create pollset. Falling back to non-pollset mode." << LL_ENDL; + sPollSet = NULL; + } + else + { + LL_DEBUGS("PluginPoll") << "created pollset " << sPollSet << LL_ENDL; + + // Pollset was created, add all instances to it. + for(iter = sInstances.begin(); iter != sInstances.end(); iter++) + { + if((*iter)->mPollFD.client_data) + { + status = apr_pollset_add(sPollSet, &((*iter)->mPollFD)); + if(status == APR_SUCCESS) + { + (*iter)->mPolledInput = true; + } + else + { + LL_WARNS("PluginPoll") << "apr_pollset_add failed with status " << status << LL_ENDL; + } + } + } + } + } + } + sInstancesMutex->unlock(); +} + +void LLPluginProcessParent::setUseReadThread(bool use_read_thread) +{ + if(sUseReadThread != use_read_thread) + { + sUseReadThread = use_read_thread; + + if(sUseReadThread) + { + if(!sPollThread) + { + // start up the read thread + LL_INFOS("PluginPoll") << "creating polling thread " << LL_ENDL; + + // make sure the pollset gets rebuilt. + sPollsetNeedsRebuild = true; + + sPollThread = new LLPluginProcessParentPollThread; + sPollThread->start(); + } + } + else + { + if(sPollThread) + { + // shut down the read thread + LL_INFOS("PluginPoll") << "destroying polling thread " << LL_ENDL; + delete sPollThread; + sPollThread = NULL; + } + } + + } +} + +void LLPluginProcessParent::poll(F64 timeout) +{ + if(sPollsetNeedsRebuild || !sUseReadThread) + { + sPollsetNeedsRebuild = false; + updatePollset(); + } + + if(sPollSet) + { + apr_status_t status; + apr_int32_t count; + const apr_pollfd_t *descriptors; + status = apr_pollset_poll(sPollSet, (apr_interval_time_t)(timeout * 1000000), &count, &descriptors); + if(status == APR_SUCCESS) + { + // One or more of the descriptors signalled. Call them. + for(int i = 0; i < count; i++) + { + LLPluginProcessParent *self = (LLPluginProcessParent *)(descriptors[i].client_data); + // NOTE: the descriptor returned here is actually a COPY of the original (even though we create the pollset with APR_POLLSET_NOCOPY). + // This means that even if the parent has set its mPollFD.client_data to NULL, the old pointer may still there in this descriptor. + // It's even possible that the old pointer no longer points to a valid LLPluginProcessParent. + // This means that we can't safely dereference the 'self' pointer here without some extra steps... + if(self) + { + // Make sure this pointer is still in the instances list + sInstancesMutex->lock(); + bool valid = false; + for(std::list::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) + { + if(*iter == self) + { + // Lock the instance's mutex before unlocking the global mutex. + // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. + self->mIncomingQueueMutex.lock(); + valid = true; + break; + } + } + sInstancesMutex->unlock(); + + if(valid) + { + // The instance is still valid. + // Pull incoming messages off the socket + self->servicePoll(); + self->mIncomingQueueMutex.unlock(); + } + else + { + LL_DEBUGS("PluginPoll") << "detected deleted instance " << self << LL_ENDL; + } + + } + } + } + else if(APR_STATUS_IS_TIMEUP(status)) + { + // timed out with no incoming data. Just return. + } + else if(status == EBADF) + { + // This happens when one of the file descriptors in the pollset is destroyed, which happens whenever a plugin's socket is closed. + // The pollset has been or will be recreated, so just return. + LL_DEBUGS("PluginPoll") << "apr_pollset_poll returned EBADF" << LL_ENDL; + } + else if(status != APR_SUCCESS) + { + LL_WARNS("PluginPoll") << "apr_pollset_poll failed with status " << status << LL_ENDL; + } + } +} + +void LLPluginProcessParent::servicePoll() +{ + bool result = true; + + // poll signalled on this object's socket. Try to process incoming messages. + if(mMessagePipe) + { + result = mMessagePipe->pumpInput(0.0f); + } + + if(!result) + { + // If we got a read error on input, remove this pipe from the pollset + apr_pollset_remove(sPollSet, &mPollFD); + + // and tell the code not to re-add it + mPollFD.client_data = NULL; + } +} void LLPluginProcessParent::receiveMessageRaw(const std::string &message) { LL_DEBUGS("Plugin") << "Received: " << message << LL_ENDL; - - // FIXME: should this go into a queue instead? LLPluginMessage parsed; if(parsed.parse(message) != -1) { - receiveMessage(parsed); + if(parsed.hasValue("blocking_request")) + { + mBlocked = true; + } + + if(mPolledInput) + { + // This is being called on the polling thread -- only do minimal processing/queueing. + receiveMessageEarly(parsed); + } + else + { + // This is not being called on the polling thread -- do full message processing at this time. + receiveMessage(parsed); + } } } -void LLPluginProcessParent::receiveMessage(const LLPluginMessage &message) +void LLPluginProcessParent::receiveMessageEarly(const LLPluginMessage &message) { - if(message.hasValue("blocking_request")) + // NOTE: this function will be called from the polling thread. It will be called with mIncomingQueueMutex _already locked_. + + bool handled = false; + + std::string message_class = message.getClass(); + if(message_class == LLPLUGIN_MESSAGE_CLASS_INTERNAL) { - mBlocked = true; + // no internal messages need to be handled early. } + else + { + // Call out to the owner and see if they to reply + // TODO: Should this only happen when blocked? + if(mOwner != NULL) + { + handled = mOwner->receivePluginMessageEarly(message); + } + } + + if(!handled) + { + // any message that wasn't handled early needs to be queued. +// mIncomingQueueMutex.lock(); + mIncomingQueue.push(message); +// mIncomingQueueMutex.unlock(); + } +} +void LLPluginProcessParent::receiveMessage(const LLPluginMessage &message) +{ std::string message_class = message.getClass(); if(message_class == LLPLUGIN_MESSAGE_CLASS_INTERNAL) { @@ -704,12 +1070,12 @@ bool LLPluginProcessParent::pluginLockedUpOrQuit() if(!mProcess.isRunning()) { - LL_WARNS("Plugin") << "child exited" << llendl; + LL_WARNS("Plugin") << "child exited" << LL_ENDL; result = true; } else if(pluginLockedUp()) { - LL_WARNS("Plugin") << "timeout" << llendl; + LL_WARNS("Plugin") << "timeout" << LL_ENDL; result = true; } diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index 31f125bfb3..1ad0fbf059 100644 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -41,12 +41,14 @@ #include "llpluginsharedmemory.h" #include "lliosocket.h" +#include "llthread.h" class LLPluginProcessParentOwner { public: virtual ~LLPluginProcessParentOwner(); virtual void receivePluginMessage(const LLPluginMessage &message) = 0; + virtual bool receivePluginMessageEarly(const LLPluginMessage &message) {return false;}; // This will only be called when the plugin has died unexpectedly virtual void pluginLaunchFailed() {}; virtual void pluginDied() {}; @@ -90,7 +92,9 @@ public: void receiveMessage(const LLPluginMessage &message); // Inherited from LLPluginMessagePipeOwner - void receiveMessageRaw(const std::string &message); + /*virtual*/ void receiveMessageRaw(const std::string &message); + /*virtual*/ void receiveMessageEarly(const LLPluginMessage &message); + /*virtual*/ void setMessagePipe(LLPluginMessagePipe *message_pipe) ; // This adds a memory segment shared with the client, generating a name for the segment. The name generated is guaranteed to be unique on the host. // The caller must call removeSharedMemory first (and wait until getSharedMemorySize returns 0 for the indicated name) before re-adding a segment with the same name. @@ -113,7 +117,11 @@ public: void setLockupTimeout(F32 timeout) { mPluginLockupTimeout = timeout; }; F64 getCPUUsage() { return mCPUUsage; }; - + + static void poll(F64 timeout); + static bool canPollThreadRun() { return (sPollSet || sPollsetNeedsRebuild || sUseReadThread); }; + static void setUseReadThread(bool use_read_thread); + static bool getUseReadThread() { return sUseReadThread; }; private: enum EState @@ -164,12 +172,26 @@ private: bool mDisableTimeout; bool mDebug; bool mBlocked; + bool mPolledInput; LLProcessLauncher mDebugger; F32 mPluginLaunchTimeout; // Somewhat longer timeout for initial launch. F32 mPluginLockupTimeout; // If we don't receive a heartbeat in this many seconds, we declare the plugin locked up. + static bool sUseReadThread; + apr_pollfd_t mPollFD; + static apr_pollset_t *sPollSet; + static bool sPollsetNeedsRebuild; + static LLMutex *sInstancesMutex; + static std::list sInstances; + static void dirtyPollSet(); + static void updatePollset(); + void servicePoll(); + static LLThread *sPollThread; + + LLMutex mIncomingQueueMutex; + std::queue mIncomingQueue; }; #endif // LL_LLPLUGINPROCESSPARENT_H -- cgit v1.2.3 From b7bc6920c89791563e4fd63002e632967ffde966 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Tue, 27 Apr 2010 17:43:09 -0700 Subject: Fixed a silly error in the code that tries to avoid queueing up mouse move events to blocked plugins. --- indra/llplugin/llpluginclassmedia.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index 24c671437e..0c9b325b68 100644 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -440,6 +440,7 @@ void LLPluginClassMedia::mouseEvent(EMouseEventType type, int button, int x, int if(!mPlugin || !mPlugin->isRunning() || mPlugin->isBlocked()) { // Don't queue up mouse move events that can't be delivered. + return; } if((x == mLastMouseX) && (y == mLastMouseY)) -- cgit v1.2.3 From 77b13dc2df679c46f648a4f99c8e4d8836983a48 Mon Sep 17 00:00:00 2001 From: Monroe Linden Date: Thu, 29 Apr 2010 17:33:37 -0700 Subject: Incorporate suggestions from Richard's review of the LLPlugin changes. Use LLMutexLock (stack-based locker/unlocker) for the straightforward cases instead of explicit lock()/unlock(). There are still a couple of cases (one overlapping lock lifetime and two loops that unlock the mutex to call another function inside the loop) where I'm leaving explicit lock/unlock calls. Rename LLPluginProcessParent::sPollThread to sReadThread, for consistency. Made the LLPluginProcessParent destructor hold mIncomingQueueMutex while removing the instance from the global list -- this should prevent a possible race condition in LLPluginProcessParent::poll(). Removed a redundant check when calling LLPluginProcessParent::setUseReadThread(). --- indra/llplugin/llpluginmessagepipe.cpp | 9 ++-- indra/llplugin/llpluginprocessparent.cpp | 78 +++++++++++++++++--------------- indra/llplugin/llpluginprocessparent.h | 2 +- 3 files changed, 46 insertions(+), 43 deletions(-) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginmessagepipe.cpp b/indra/llplugin/llpluginmessagepipe.cpp index a62534de95..89f8b44569 100644 --- a/indra/llplugin/llpluginmessagepipe.cpp +++ b/indra/llplugin/llpluginmessagepipe.cpp @@ -117,10 +117,9 @@ LLPluginMessagePipe::~LLPluginMessagePipe() bool LLPluginMessagePipe::addMessage(const std::string &message) { // queue the message for later output - mOutputMutex.lock(); + LLMutexLock lock(&mOutputMutex); mOutput += message; mOutput += MESSAGE_DELIMITER; // message separator - mOutputMutex.unlock(); return true; } @@ -173,7 +172,7 @@ bool LLPluginMessagePipe::pumpOutput() apr_status_t status; apr_size_t size; - mOutputMutex.lock(); + LLMutexLock lock(&mOutputMutex); if(!mOutput.empty()) { // write any outgoing messages @@ -225,7 +224,6 @@ bool LLPluginMessagePipe::pumpOutput() result = false; } } - mOutputMutex.unlock(); } return result; @@ -288,9 +286,8 @@ bool LLPluginMessagePipe::pumpInput(F64 timeout) if(size > 0) { - mInputMutex.lock(); + LLMutexLock lock(&mInputMutex); mInput.append(input_buf, size); - mInputMutex.unlock(); } if(status == APR_SUCCESS) diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index c61a903a2c..9d510e737d 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -50,7 +50,7 @@ apr_pollset_t *LLPluginProcessParent::sPollSet = NULL; bool LLPluginProcessParent::sPollsetNeedsRebuild = false; LLMutex *LLPluginProcessParent::sInstancesMutex; std::list LLPluginProcessParent::sInstances; -LLThread *LLPluginProcessParent::sPollThread = NULL; +LLThread *LLPluginProcessParent::sReadThread = NULL; class LLPluginProcessParentPollThread: public LLThread @@ -108,9 +108,10 @@ LLPluginProcessParent::LLPluginProcessParent(LLPluginProcessParentOwner *owner): mHeartbeat.stop(); // Don't add to the global list until fully constructed. - sInstancesMutex->lock(); - sInstances.push_back(this); - sInstancesMutex->unlock(); + { + LLMutexLock lock(sInstancesMutex); + sInstances.push_back(this); + } } LLPluginProcessParent::~LLPluginProcessParent() @@ -118,9 +119,14 @@ LLPluginProcessParent::~LLPluginProcessParent() LL_DEBUGS("Plugin") << "destructor" << LL_ENDL; // Remove from the global list before beginning destruction. - sInstancesMutex->lock(); - sInstances.remove(this); - sInstancesMutex->unlock(); + { + // Make sure to get the global mutex _first_ here, to avoid a possible deadlock against LLPluginProcessParent::poll() + LLMutexLock lock(sInstancesMutex); + { + LLMutexLock lock2(&mIncomingQueueMutex); + sInstances.remove(this); + } + } // Destroy any remaining shared memory regions sharedMemoryRegionsType::iterator iter; @@ -139,9 +145,11 @@ LLPluginProcessParent::~LLPluginProcessParent() void LLPluginProcessParent::killSockets(void) { - mIncomingQueueMutex.lock(); - killMessagePipe(); - mIncomingQueueMutex.unlock(); + { + LLMutexLock lock(&mIncomingQueueMutex); + killMessagePipe(); + } + mListenSocket.reset(); mSocket.reset(); } @@ -619,10 +627,10 @@ void LLPluginProcessParent::dirtyPollSet() { sPollsetNeedsRebuild = true; - if(sPollThread) + if(sReadThread) { - LL_DEBUGS("PluginPoll") << "unpausing polling thread " << LL_ENDL; - sPollThread->unpause(); + LL_DEBUGS("PluginPoll") << "unpausing read thread " << LL_ENDL; + sReadThread->unpause(); } } @@ -634,7 +642,7 @@ void LLPluginProcessParent::updatePollset() return; } - sInstancesMutex->lock(); + LLMutexLock lock(sInstancesMutex); if(sPollSet) { @@ -658,7 +666,7 @@ void LLPluginProcessParent::updatePollset() } } - if(sUseReadThread && sPollThread && !sPollThread->isQuitting()) + if(sUseReadThread && sReadThread && !sReadThread->isQuitting()) { if(!sPollSet && (count > 0)) { @@ -692,7 +700,6 @@ void LLPluginProcessParent::updatePollset() } } } - sInstancesMutex->unlock(); } void LLPluginProcessParent::setUseReadThread(bool use_read_thread) @@ -703,26 +710,26 @@ void LLPluginProcessParent::setUseReadThread(bool use_read_thread) if(sUseReadThread) { - if(!sPollThread) + if(!sReadThread) { // start up the read thread - LL_INFOS("PluginPoll") << "creating polling thread " << LL_ENDL; + LL_INFOS("PluginPoll") << "creating read thread " << LL_ENDL; // make sure the pollset gets rebuilt. sPollsetNeedsRebuild = true; - sPollThread = new LLPluginProcessParentPollThread; - sPollThread->start(); + sReadThread = new LLPluginProcessParentPollThread; + sReadThread->start(); } } else { - if(sPollThread) + if(sReadThread) { // shut down the read thread - LL_INFOS("PluginPoll") << "destroying polling thread " << LL_ENDL; - delete sPollThread; - sPollThread = NULL; + LL_INFOS("PluginPoll") << "destroying read thread " << LL_ENDL; + delete sReadThread; + sReadThread = NULL; } } @@ -756,21 +763,22 @@ void LLPluginProcessParent::poll(F64 timeout) if(self) { // Make sure this pointer is still in the instances list - sInstancesMutex->lock(); bool valid = false; - for(std::list::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) { - if(*iter == self) + LLMutexLock lock(sInstancesMutex); + for(std::list::iterator iter = sInstances.begin(); iter != sInstances.end(); ++iter) { - // Lock the instance's mutex before unlocking the global mutex. - // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. - self->mIncomingQueueMutex.lock(); - valid = true; - break; + if(*iter == self) + { + // Lock the instance's mutex before unlocking the global mutex. + // This avoids a possible race condition where the instance gets deleted between this check and the servicePoll() call. + self->mIncomingQueueMutex.lock(); + valid = true; + break; + } } } - sInstancesMutex->unlock(); - + if(valid) { // The instance is still valid. @@ -872,9 +880,7 @@ void LLPluginProcessParent::receiveMessageEarly(const LLPluginMessage &message) if(!handled) { // any message that wasn't handled early needs to be queued. -// mIncomingQueueMutex.lock(); mIncomingQueue.push(message); -// mIncomingQueueMutex.unlock(); } } diff --git a/indra/llplugin/llpluginprocessparent.h b/indra/llplugin/llpluginprocessparent.h index 1ad0fbf059..4dff835b6a 100644 --- a/indra/llplugin/llpluginprocessparent.h +++ b/indra/llplugin/llpluginprocessparent.h @@ -188,7 +188,7 @@ private: static void dirtyPollSet(); static void updatePollset(); void servicePoll(); - static LLThread *sPollThread; + static LLThread *sReadThread; LLMutex mIncomingQueueMutex; std::queue mIncomingQueue; -- cgit v1.2.3 From 34e29fe71277609e6c18c967b8fd7a45116d4d82 Mon Sep 17 00:00:00 2001 From: Xiaohong Bao Date: Thu, 29 Apr 2010 12:24:46 -0600 Subject: add debug code for EXT-7011: crash at LLPluginClassMedia::idle [secondlife-bin llpluginclassmedia.cpp:158] --- indra/llplugin/llpluginclassmedia.cpp | 4 ++++ indra/llplugin/llpluginclassmedia.h | 8 ++++++++ 2 files changed, 12 insertions(+) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginclassmedia.cpp b/indra/llplugin/llpluginclassmedia.cpp index 0c9b325b68..41ace62964 100644 --- a/indra/llplugin/llpluginclassmedia.cpp +++ b/indra/llplugin/llpluginclassmedia.cpp @@ -57,11 +57,15 @@ LLPluginClassMedia::LLPluginClassMedia(LLPluginClassMediaOwner *owner) mOwner = owner; mPlugin = NULL; reset(); + + //debug use + mDeleteOK = true ; } LLPluginClassMedia::~LLPluginClassMedia() { + llassert_always(mDeleteOK) ; reset(); } diff --git a/indra/llplugin/llpluginclassmedia.h b/indra/llplugin/llpluginclassmedia.h index 8c7b00f45b..66853c9940 100644 --- a/indra/llplugin/llpluginclassmedia.h +++ b/indra/llplugin/llpluginclassmedia.h @@ -373,6 +373,14 @@ protected: F64 mCurrentRate; F64 mLoadedDuration; +//-------------------------------------- + //debug use only + // +private: + bool mDeleteOK ; +public: + void setDeleteOK(bool flag) { mDeleteOK = flag ;} +//-------------------------------------- }; #endif // LL_LLPLUGINCLASSMEDIA_H -- cgit v1.2.3 From d597a7e36a0a8c4cbee70b053c41aa01afeab6b5 Mon Sep 17 00:00:00 2001 From: Tofu Linden Date: Fri, 30 Apr 2010 08:27:56 +0100 Subject: Looks like the linux apr version is a bit too old to build 5458d58b4cd0. Use this workaround for now. --- indra/llplugin/llpluginprocessparent.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'indra/llplugin') diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index 9d510e737d..3589b22a77 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -670,12 +670,15 @@ void LLPluginProcessParent::updatePollset() { if(!sPollSet && (count > 0)) { +#ifdef APR_POLLSET_NOCOPY // The pollset doesn't exist yet. Create it now. apr_status_t status = apr_pollset_create(&sPollSet, count, gAPRPoolp, APR_POLLSET_NOCOPY); if(status != APR_SUCCESS) { +#endif // APR_POLLSET_NOCOPY LL_WARNS("PluginPoll") << "Couldn't create pollset. Falling back to non-pollset mode." << LL_ENDL; sPollSet = NULL; +#ifdef APR_POLLSET_NOCOPY } else { @@ -698,6 +701,7 @@ void LLPluginProcessParent::updatePollset() } } } +#endif // APR_POLLSET_NOCOPY } } } -- cgit v1.2.3