diff options
Diffstat (limited to 'indra/llcommon')
| -rw-r--r-- | indra/llcommon/CMakeLists.txt | 8 | ||||
| -rw-r--r-- | indra/llcommon/llcoros.cpp | 29 | ||||
| -rw-r--r-- | indra/llcommon/lldependencies.cpp | 4 | ||||
| -rw-r--r-- | indra/llcommon/llerror.h | 87 | ||||
| -rw-r--r-- | indra/llcommon/lleventcoro.cpp | 4 | ||||
| -rw-r--r-- | indra/llcommon/llevents.cpp | 18 | ||||
| -rw-r--r-- | indra/llcommon/llevents.h | 32 | ||||
| -rw-r--r-- | indra/llcommon/llexception.cpp | 55 | ||||
| -rw-r--r-- | indra/llcommon/llexception.h | 46 | ||||
| -rw-r--r-- | indra/llcommon/llfasttimer.h | 11 | ||||
| -rw-r--r-- | indra/llcommon/llleap.cpp | 6 | ||||
| -rw-r--r-- | indra/llcommon/llmemory.h | 10 | ||||
| -rw-r--r-- | indra/llcommon/llprocess.cpp | 20 | ||||
| -rw-r--r-- | indra/llcommon/llthreadsafequeue.cpp | 14 | ||||
| -rw-r--r-- | indra/llcommon/lluuid.cpp | 2 | ||||
| -rw-r--r-- | indra/llcommon/tests/llerror_test.cpp | 17 | ||||
| -rw-r--r-- | indra/llcommon/tests/llexception_test.cpp | 308 | ||||
| -rw-r--r-- | indra/llcommon/tests/wrapllerrs.h | 3 |
18 files changed, 590 insertions, 84 deletions
diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 44f45144e5..410a5819b3 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -58,6 +58,7 @@ set(llcommon_SOURCE_FILES lleventfilter.cpp llevents.cpp lleventtimer.cpp + llexception.cpp llfasttimer.cpp llfile.cpp llfindlocale.cpp @@ -316,7 +317,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llsdserialize "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llsingleton "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llsingleton "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llstring "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltrace "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltreeiterators "" "${test_libs}") @@ -329,6 +330,11 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llstreamqueue "" "${test_libs}") +## llexception_test.cpp isn't a regression test, and doesn't need to be run +## every build. It's to help a developer make implementation choices about +## throwing and catching exceptions. +##LL_ADD_INTEGRATION_TEST(llexception "" "${test_libs}") + # *TODO - reenable these once tcmalloc libs no longer break the build. #ADD_BUILD_TEST(llallocator llcommon) #ADD_BUILD_TEST(llallocator_heap_profile llcommon) diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index d16bf0160b..8e516d8beb 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -38,6 +38,7 @@ #include "llevents.h" #include "llerror.h" #include "stringize.h" +#include "llexception.h" // do nothing, when we need nothing done void LLCoros::no_cleanup(CoroData*) {} @@ -131,9 +132,9 @@ bool LLCoros::cleanup(const LLSD&) if ((previousCount < 5) || !(previousCount % 50)) { if (previousCount < 5) - LL_INFOS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << LL_ENDL; + LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << LL_ENDL; else - LL_INFOS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << "("<< previousCount << ")" << LL_ENDL; + LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << "("<< previousCount << ")" << LL_ENDL; } // The erase() call will invalidate its passed iterator value -- @@ -185,9 +186,9 @@ std::string LLCoros::generateDistinctName(const std::string& prefix) const if ((previousCount < 5) || !(previousCount % 50)) { if (previousCount < 5) - LL_INFOS("LLCoros") << "LLCoros: launching coroutine " << name << LL_ENDL; + LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << LL_ENDL; else - LL_INFOS("LLCoros") << "LLCoros: launching coroutine " << name << "(" << previousCount << ")" << LL_ENDL; + LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << "(" << previousCount << ")" << LL_ENDL; } @@ -223,7 +224,7 @@ std::string LLCoros::getName() const void LLCoros::setStackSize(S32 stacksize) { - LL_INFOS("LLCoros") << "Setting coroutine stack size to " << stacksize << LL_ENDL; + LL_DEBUGS("LLCoros") << "Setting coroutine stack size to " << stacksize << LL_ENDL; mStackSize = stacksize; } @@ -235,7 +236,23 @@ void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& calla // capture the 'self' param in CoroData data->mSelf = &self; // run the code the caller actually wants in the coroutine - callable(); + try + { + callable(); + } + catch (const LLContinueError&) + { + // Any uncaught exception derived from LLContinueError will be caught + // here and logged. This coroutine will terminate but the rest of the + // viewer will carry on. + LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + } + catch (...) + { + // Any OTHER kind of uncaught exception will cause the viewer to + // crash, hopefully informatively. + CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + } // This cleanup isn't perfectly symmetrical with the way we initially set // data->mPrev, but this is our last chance to reset mCurrentCoro. sCurrentCoro.reset(data->mPrev); diff --git a/indra/llcommon/lldependencies.cpp b/indra/llcommon/lldependencies.cpp index 87a699ff14..0d5757effd 100644 --- a/indra/llcommon/lldependencies.cpp +++ b/indra/llcommon/lldependencies.cpp @@ -39,8 +39,8 @@ #include <boost/graph/adjacency_list.hpp> #include <boost/graph/topological_sort.hpp> #include <boost/graph/exception.hpp> -#include <boost/throw_exception.hpp> // other Linden headers +#include "llexception.h" LLDependenciesBase::VertexList LLDependenciesBase::topo_sort(int vertices, const EdgeList& edges) const { @@ -77,7 +77,7 @@ LLDependenciesBase::VertexList LLDependenciesBase::topo_sort(int vertices, const // Omit independent nodes: display only those that might contribute to // the cycle. describe(out, false); - BOOST_THROW_EXCEPTION(Cycle(out.str())); + LLTHROW(Cycle(out.str())); } // A peculiarity of boost::topological_sort() is that it emits results in // REVERSE topological order: to get the result you want, you must diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 3beef65723..7cbe4334b3 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -174,7 +174,8 @@ namespace LLError // not really a level // used to indicate that no messages should be logged }; - + // If you change ELevel, please update llvlog() macro below. + /* Macro support The classes CallSite and Log are used by the logging macros below. They are not intended for general use. @@ -305,24 +306,38 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; ///////////////////////////////// // Error Logging Macros -// See top of file for common usage. +// See top of file for common usage. ///////////////////////////////// -// this macro uses a one-shot do statement to avoid parsing errors when writing control flow statements -// without braces: -// if (condition) LL_INFOS() << "True" << LL_ENDL; else LL_INFOS()() << "False" << LL_ENDL - -#define lllog(level, once, ...) \ - do { \ - const char* tags[] = {"", ##__VA_ARGS__}; \ - ::size_t tag_count = LL_ARRAY_SIZE(tags) - 1; \ - static LLError::CallSite _site( \ - level, __FILE__, __LINE__, typeid(_LL_CLASS_TO_LOG), __FUNCTION__, once, &tags[1], tag_count);\ - if (LL_UNLIKELY(_site.shouldLog())) \ - { \ - std::ostringstream* _out = LLError::Log::out(); \ +// Instead of using LL_DEBUGS(), LL_INFOS() et al., it may be tempting to +// directly code the lllog() macro so you can pass in the LLError::ELevel as a +// variable. DON'T DO IT! The reason is that the first time control passes +// through lllog(), it initializes a local static LLError::CallSite with that +// *first* ELevel value. All subsequent visits will decide whether or not to +// emit output based on the *first* ELevel value bound into that static +// CallSite instance. Use LL_VLOGS() instead. lllog() assumes its ELevel +// argument never varies. + +// this macro uses a one-shot do statement to avoid parsing errors when +// writing control flow statements without braces: +// if (condition) LL_INFOS() << "True" << LL_ENDL; else LL_INFOS()() << "False" << LL_ENDL; + +#define lllog(level, once, ...) \ + do { \ + const char* tags[] = {"", ##__VA_ARGS__}; \ + static LLError::CallSite _site(lllog_site_args_(level, once, tags)); \ + lllog_test_() + +#define lllog_test_() \ + if (LL_UNLIKELY(_site.shouldLog())) \ + { \ + std::ostringstream* _out = LLError::Log::out(); \ (*_out) +#define lllog_site_args_(level, once, tags) \ + level, __FILE__, __LINE__, typeid(_LL_CLASS_TO_LOG), \ + __FUNCTION__, once, &tags[1], LL_ARRAY_SIZE(tags)-1 + //Use this construct if you need to do computation in the middle of a //message: // @@ -363,4 +378,46 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_INFOS_ONCE(...) lllog(LLError::LEVEL_INFO, true, ##__VA_ARGS__) #define LL_WARNS_ONCE(...) lllog(LLError::LEVEL_WARN, true, ##__VA_ARGS__) +// Use this if you need to pass LLError::ELevel as a variable. +#define LL_VLOGS(level, ...) llvlog(level, false, ##__VA_ARGS__) +#define LL_VLOGS_ONCE(level, ...) llvlog(level, true, ##__VA_ARGS__) + +// The problem with using lllog() with a variable level is that the first time +// through, it initializes a static CallSite instance with whatever level you +// pass. That first level is bound into the CallSite; the level parameter is +// never again examined. One approach to variable level would be to +// dynamically construct a CallSite instance every call -- which could get +// expensive, depending on context. So instead, initialize a static CallSite +// for each level value we support, then dynamically select the CallSite +// instance for the passed level value. +// Compare implementation to lllog() above. +#define llvlog(level, once, ...) \ + do { \ + const char* tags[] = {"", ##__VA_ARGS__}; \ + /* Need a static CallSite instance per expected ELevel value. */ \ + /* Since we intend to index this array with the ELevel, */ \ + /* _sites[0] should be ELevel(0), and so on -- avoid using */ \ + /* ELevel symbolic names when initializing -- except for */ \ + /* the last entry, which handles anything beyond the end. */ \ + /* (Commented ELevel value names are from 2016-09-01.) */ \ + /* Passing an ELevel past the end of this array is itself */ \ + /* a fatal error, so ensure the last is LEVEL_ERROR. */ \ + static LLError::CallSite _sites[] = \ + { \ + /* LEVEL_DEBUG */ \ + LLError::CallSite(lllog_site_args_(LLError::ELevel(0), once, tags)), \ + /* LEVEL_INFO */ \ + LLError::CallSite(lllog_site_args_(LLError::ELevel(1), once, tags)), \ + /* LEVEL_WARN */ \ + LLError::CallSite(lllog_site_args_(LLError::ELevel(2), once, tags)), \ + /* LEVEL_ERROR */ \ + LLError::CallSite(lllog_site_args_(LLError::LEVEL_ERROR, once, tags)) \ + }; \ + /* Clamp the passed 'level' to at most last entry */ \ + std::size_t which((std::size_t(level) >= LL_ARRAY_SIZE(_sites)) ? \ + (LL_ARRAY_SIZE(_sites) - 1) : std::size_t(level)); \ + /* selected CallSite *must* be named _site for LL_ENDL */ \ + LLError::CallSite& _site(_sites[which]); \ + lllog_test_() + #endif // LL_LLERROR_H diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index f444530a17..56367b8f54 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -34,12 +34,12 @@ #include <map> // std headers // external library headers -#include <boost/throw_exception.hpp> // other Linden headers #include "llsdserialize.h" #include "llerror.h" #include "llcoros.h" #include "llmake.h" +#include "llexception.h" #include "lleventfilter.h" @@ -352,7 +352,7 @@ LLSD errorException(const LLEventWithID& result, const std::string& desc) // returning it, deliver it via exception. if (result.second) { - BOOST_THROW_EXCEPTION(LLErrorEvent(desc, result.first)); + LLTHROW(LLErrorEvent(desc, result.first)); } // That way, our caller knows a simple return must be from the reply // pump (pump 0). diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 50919edb8e..19d700a3b0 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -45,7 +45,6 @@ #include <cctype> // external library headers #include <boost/range/iterator_range.hpp> -#include <boost/throw_exception.hpp> #if LL_WINDOWS #pragma warning (push) #pragma warning (disable : 4701) // compiler thinks might use uninitialized var, but no @@ -58,6 +57,7 @@ #include "stringize.h" #include "llerror.h" #include "llsdutil.h" +#include "llexception.h" #if LL_MSVC #pragma warning (disable : 4702) #endif @@ -175,7 +175,7 @@ std::string LLEventPumps::registerNew(const LLEventPump& pump, const std::string // Unless we're permitted to tweak it, that's Bad. if (! tweak) { - BOOST_THROW_EXCEPTION(LLEventPump::DupPumpName(std::string("Duplicate LLEventPump name '") + name + "'")); + LLTHROW(LLEventPump::DupPumpName("Duplicate LLEventPump name '" + name + "'")); } // The passed name isn't unique, but we're permitted to tweak it. Find the // first decimal-integer suffix not already taken. The insert() attempt @@ -327,9 +327,8 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL // is only when the existing connection object is still connected. if (found != mConnections.end() && found->second.connected()) { - BOOST_THROW_EXCEPTION( - DupListenerName(std::string("Attempt to register duplicate listener name '") + name + - "' on " + typeid(*this).name() + " '" + getName() + "'")); + LLTHROW(DupListenerName("Attempt to register duplicate listener name '" + name + + "' on " + typeid(*this).name() + " '" + getName() + "'")); } // Okay, name is unique, try to reconcile its dependencies. Specify a new // "node" value that we never use for an mSignal placement; we'll fix it @@ -355,9 +354,8 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL // unsortable. If we leave the new node in mDeps, it will continue // to screw up all future attempts to sort()! Pull it out. mDeps.remove(name); - BOOST_THROW_EXCEPTION( - Cycle(std::string("New listener '") + name + "' on " + typeid(*this).name() + - " '" + getName() + "' would cause cycle: " + e.what())); + LLTHROW(Cycle("New listener '" + name + "' on " + typeid(*this).name() + + " '" + getName() + "' would cause cycle: " + e.what())); } // Walk the list to verify that we haven't changed the order. float previous = 0.0, myprev = 0.0; @@ -421,7 +419,7 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL // NOW remove the offending listener node. mDeps.remove(name); // Having constructed a description of the order change, inform caller. - BOOST_THROW_EXCEPTION(OrderChange(out.str())); + LLTHROW(OrderChange(out.str())); } // This node becomes the previous one. previous = dmi->second; @@ -611,7 +609,7 @@ bool LLListenerOrPumpName::operator()(const LLSD& event) const { if (! mListener) { - BOOST_THROW_EXCEPTION(Empty("attempting to call uninitialized")); + LLTHROW(Empty("attempting to call uninitialized")); } return (*mListener)(event); } diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 8ff337911d..1526128725 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -95,12 +95,32 @@ struct LLStopWhenHandled result_type operator()(InputIterator first, InputIterator last) const { for (InputIterator si = first; si != last; ++si) - { - if (*si) - { - return true; - } - } + { + try + { + if (*si) + { + return true; + } + } + catch (const LLContinueError&) + { + // We catch LLContinueError here because an LLContinueError- + // based exception means the viewer as a whole should carry on + // to the best of our ability. Therefore subsequent listeners + // on the same LLEventPump should still receive this event. + + // The iterator passed to a boost::signals2 Combiner is very + // clever, but provides no contextual information. We would + // very much like to be able to log the name of the LLEventPump + // plus the name of this particular listener, but alas. + LOG_UNHANDLED_EXCEPTION("LLEventPump"); + } + // We do NOT catch (...) here because we might as well let it + // propagate out to the generic handler. If we were able to log + // context information here, that would be great, but we can't, so + // there's no point. + } return false; } }; diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp new file mode 100644 index 0000000000..b32ec2c9c9 --- /dev/null +++ b/indra/llcommon/llexception.cpp @@ -0,0 +1,55 @@ +/** + * @file llexception.cpp + * @author Nat Goodspeed + * @date 2016-08-12 + * @brief Implementation for llexception. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llexception.h" +// STL headers +// std headers +#include <typeinfo> +// external library headers +#include <boost/exception/diagnostic_information.hpp> +// other Linden headers +#include "llerror.h" +#include "llerrorcontrol.h" + +namespace { +// used by crash_on_unhandled_exception_() and log_unhandled_exception_() +void log_unhandled_exception_(LLError::ELevel level, + const char* file, int line, const char* pretty_function, + const std::string& context) +{ + // log same message but allow caller-specified severity level + LL_VLOGS(level, "LLException") << LLError::abbreviateFile(file) + << "(" << line << "): Unhandled exception caught in " << pretty_function; + if (! context.empty()) + { + LL_CONT << ": " << context; + } + LL_CONT << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; +} +} + +void crash_on_unhandled_exception_(const char* file, int line, const char* pretty_function, + const std::string& context) +{ + // LL_ERRS() terminates and propagates message into crash dump. + log_unhandled_exception_(LLError::LEVEL_ERROR, file, line, pretty_function, context); +} + +void log_unhandled_exception_(const char* file, int line, const char* pretty_function, + const std::string& context) +{ + // Use LL_WARNS() because we seriously do not expect this to happen + // routinely, but we DO expect to return from this function. + log_unhandled_exception_(LLError::LEVEL_WARN, file, line, pretty_function, context); +} diff --git a/indra/llcommon/llexception.h b/indra/llcommon/llexception.h index 3ac2f4762f..dfcb7c192f 100644 --- a/indra/llcommon/llexception.h +++ b/indra/llcommon/llexception.h @@ -14,22 +14,24 @@ #include <stdexcept> #include <boost/exception/exception.hpp> +#include <boost/throw_exception.hpp> +#include <boost/current_function.hpp> + +// "Found someone who can comfort me +// But there are always exceptions..." +// - Empty Pages, Traffic, from John Barleycorn (1970) +// https://www.youtube.com/watch?v=dRH0CGVK7ic /** * LLException is intended as the common base class from which all - * viewer-specific exceptions are derived. It is itself a subclass of - * boost::exception; use catch (const boost::exception& e) clause to log the - * string from boost::diagnostic_information(e). - * - * Since it is also derived from std::exception, a generic catch (const - * std::exception&) should also work, though what() is unlikely to be as - * informative as boost::diagnostic_information(). + * viewer-specific exceptions are derived. Rationale for why it's derived from + * both std::exception and boost::exception is explained in + * tests/llexception_test.cpp. * - * Please use BOOST_THROW_EXCEPTION() - * http://www.boost.org/doc/libs/release/libs/exception/doc/BOOST_THROW_EXCEPTION.html - * to throw viewer exceptions whenever possible. This enriches the exception's - * diagnostic_information() with the source file, line and containing function - * of the BOOST_THROW_EXCEPTION() macro. + * boost::current_exception_diagnostic_information() is quite wonderful: if + * all we need to do with an exception is log it, in most places we should + * catch (...) and log boost::current_exception_diagnostic_information(). + * See CRASH_ON_UNHANDLED_EXCEPTION() and LOG_UNHANDLED_EXCEPTION() below. * * There may be circumstances in which it would be valuable to distinguish an * exception explicitly thrown by viewer code from an exception thrown by @@ -60,4 +62,24 @@ struct LLContinueError: public LLException {} }; +/** + * Please use LLTHROW() to throw viewer exceptions whenever possible. This + * enriches the exception's diagnostic_information() with the source file, + * line and containing function of the LLTHROW() macro. + */ +// Currently we implement that using BOOST_THROW_EXCEPTION(). Wrap it in +// LLTHROW() in case we ever want to revisit that implementation decision. +#define LLTHROW(x) BOOST_THROW_EXCEPTION(x) + +/// Call this macro from a catch (...) clause +#define CRASH_ON_UNHANDLED_EXCEPTION(CONTEXT) \ + crash_on_unhandled_exception_(__FILE__, __LINE__, BOOST_CURRENT_FUNCTION, CONTEXT) +void crash_on_unhandled_exception_(const char*, int, const char*, const std::string&); + +/// Call this from a catch (const LLContinueError&) clause, or from a catch +/// (...) clause in which you do NOT want the viewer to crash. +#define LOG_UNHANDLED_EXCEPTION(CONTEXT) \ + log_unhandled_exception_(__FILE__, __LINE__, BOOST_CURRENT_FUNCTION, CONTEXT) +void log_unhandled_exception_(const char*, int, const char*, const std::string&); + #endif /* ! defined(LL_LLEXCEPTION_H) */ diff --git a/indra/llcommon/llfasttimer.h b/indra/llcommon/llfasttimer.h index 2370253078..f56e5596f5 100644 --- a/indra/llcommon/llfasttimer.h +++ b/indra/llcommon/llfasttimer.h @@ -296,7 +296,16 @@ LL_FORCE_INLINE BlockTimer::BlockTimer(BlockTimerStatHandle& timer) { #if LL_FAST_TIMER_ON BlockTimerStackRecord* cur_timer_data = LLThreadLocalSingletonPointer<BlockTimerStackRecord>::getInstance(); - if (!cur_timer_data) return; + if (!cur_timer_data) + { + // How likely is it that + // LLThreadLocalSingletonPointer<T>::getInstance() will return NULL? + // Even without researching, what we can say is that if we exit + // without setting mStartTime at all, gcc 4.7 produces (fatal) + // warnings about a possibly-uninitialized data member. + mStartTime = 0; + return; + } TimeBlockAccumulator& accumulator = timer.getCurrentAccumulator(); accumulator.mActiveCount++; // keep current parent as long as it is active when we are diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index a8bb9bc53a..c87d2a3e58 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -21,7 +21,6 @@ #include <boost/bind.hpp> #include <boost/scoped_ptr.hpp> #include <boost/tokenizer.hpp> -#include <boost/throw_exception.hpp> // other Linden headers #include "llerror.h" #include "llstring.h" @@ -34,6 +33,7 @@ #include "lltimer.h" #include "lluuid.h" #include "llleaplistener.h" +#include "llexception.h" #if LL_MSVC #pragma warning (disable : 4355) // 'this' used in initializer list: yes, intentionally @@ -70,7 +70,7 @@ public: // Rule out empty vector if (plugin.empty()) { - BOOST_THROW_EXCEPTION(Error("no plugin command")); + LLTHROW(Error("no plugin command")); } // Don't leave desc empty either, but in this case, if we weren't @@ -113,7 +113,7 @@ public: // If that didn't work, no point in keeping this LLLeap object. if (! mChild) { - BOOST_THROW_EXCEPTION(Error(STRINGIZE("failed to run " << mDesc))); + LLTHROW(Error(STRINGIZE("failed to run " << mDesc))); } // Okay, launch apparently worked. Change our mDonePump listener. diff --git a/indra/llcommon/llmemory.h b/indra/llcommon/llmemory.h index 0fb257aab1..575edddc43 100644 --- a/indra/llcommon/llmemory.h +++ b/indra/llcommon/llmemory.h @@ -110,11 +110,15 @@ template <typename T> T* LL_NEXT_ALIGNED_ADDRESS_64(T* address) #if defined(LL_WINDOWS) return _aligned_malloc(size, align); #else + char* aligned = NULL; void* mem = malloc( size + (align - 1) + sizeof(void*) ); - char* aligned = ((char*)mem) + sizeof(void*); - aligned += align - ((uintptr_t)aligned & (align - 1)); + if (mem) + { + aligned = ((char*)mem) + sizeof(void*); + aligned += align - ((uintptr_t)aligned & (align - 1)); - ((void**)aligned)[-1] = mem; + ((void**)aligned)[-1] = mem; + } return aligned; #endif } diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index ca19c94736..8c321d06b9 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -34,12 +34,12 @@ #include "llapr.h" #include "apr_signal.h" #include "llevents.h" +#include "llexception.h" #include <boost/foreach.hpp> #include <boost/bind.hpp> #include <boost/asio/streambuf.hpp> #include <boost/asio/buffers_iterator.hpp> -#include <boost/throw_exception.hpp> #include <iostream> #include <stdexcept> #include <limits> @@ -531,9 +531,8 @@ LLProcess::LLProcess(const LLSDOrParams& params): if (! params.validateBlock(true)) { - BOOST_THROW_EXCEPTION( - LLProcessError(STRINGIZE("not launched: failed parameter validation\n" - << LLSDNotationStreamer(params)))); + LLTHROW(LLProcessError(STRINGIZE("not launched: failed parameter validation\n" + << LLSDNotationStreamer(params)))); } mPostend = params.postend; @@ -598,11 +597,10 @@ LLProcess::LLProcess(const LLSDOrParams& params): } else { - BOOST_THROW_EXCEPTION( - LLProcessError(STRINGIZE("For " << params.executable() - << ": unsupported FileParam for " << which - << ": type='" << fparam.type() - << "', name='" << fparam.name() << "'"))); + LLTHROW(LLProcessError(STRINGIZE("For " << params.executable() + << ": unsupported FileParam for " << which + << ": type='" << fparam.type() + << "', name='" << fparam.name() << "'"))); } } // By default, pass APR_NO_PIPE for unspecified slots. @@ -681,7 +679,7 @@ LLProcess::LLProcess(const LLSDOrParams& params): if (ll_apr_warn_status(apr_proc_create(&mProcess, argv[0], &argv[0], NULL, procattr, gAPRPoolp))) { - BOOST_THROW_EXCEPTION(LLProcessError(STRINGIZE(params << " failed"))); + LLTHROW(LLProcessError(STRINGIZE(params << " failed"))); } // arrange to call status_callback() @@ -1066,7 +1064,7 @@ PIPETYPE& LLProcess::getPipe(FILESLOT slot) PIPETYPE* wp = getPipePtr<PIPETYPE>(error, slot); if (! wp) { - BOOST_THROW_EXCEPTION(NoPipe(error)); + LLTHROW(NoPipe(error)); } return *wp; } diff --git a/indra/llcommon/llthreadsafequeue.cpp b/indra/llcommon/llthreadsafequeue.cpp index a004618e96..491f920c0f 100644 --- a/indra/llcommon/llthreadsafequeue.cpp +++ b/indra/llcommon/llthreadsafequeue.cpp @@ -26,8 +26,8 @@ #include "linden_common.h" #include <apr_pools.h> #include <apr_queue.h> -#include <boost/throw_exception.hpp> #include "llthreadsafequeue.h" +#include "llexception.h" @@ -42,13 +42,13 @@ LLThreadSafeQueueImplementation::LLThreadSafeQueueImplementation(apr_pool_t * po { if(mOwnsPool) { apr_status_t status = apr_pool_create(&mPool, 0); - if(status != APR_SUCCESS) BOOST_THROW_EXCEPTION(LLThreadSafeQueueError("failed to allocate pool")); + if(status != APR_SUCCESS) LLTHROW(LLThreadSafeQueueError("failed to allocate pool")); } else { ; // No op. } apr_status_t status = apr_queue_create(&mQueue, capacity, mPool); - if(status != APR_SUCCESS) BOOST_THROW_EXCEPTION(LLThreadSafeQueueError("failed to allocate queue")); + if(status != APR_SUCCESS) LLTHROW(LLThreadSafeQueueError("failed to allocate queue")); } @@ -69,9 +69,9 @@ void LLThreadSafeQueueImplementation::pushFront(void * element) apr_status_t status = apr_queue_push(mQueue, element); if(status == APR_EINTR) { - BOOST_THROW_EXCEPTION(LLThreadSafeQueueInterrupt()); + LLTHROW(LLThreadSafeQueueInterrupt()); } else if(status != APR_SUCCESS) { - BOOST_THROW_EXCEPTION(LLThreadSafeQueueError("push failed")); + LLTHROW(LLThreadSafeQueueError("push failed")); } else { ; // Success. } @@ -89,9 +89,9 @@ void * LLThreadSafeQueueImplementation::popBack(void) apr_status_t status = apr_queue_pop(mQueue, &element); if(status == APR_EINTR) { - BOOST_THROW_EXCEPTION(LLThreadSafeQueueInterrupt()); + LLTHROW(LLThreadSafeQueueInterrupt()); } else if(status != APR_SUCCESS) { - BOOST_THROW_EXCEPTION(LLThreadSafeQueueError("pop failed")); + LLTHROW(LLThreadSafeQueueError("pop failed")); } else { return element; } diff --git a/indra/llcommon/lluuid.cpp b/indra/llcommon/lluuid.cpp index 785cf47926..d4af2c6b01 100644 --- a/indra/llcommon/lluuid.cpp +++ b/indra/llcommon/lluuid.cpp @@ -83,7 +83,7 @@ unsigned int decode( char const * fiveChars ) throw( bad_input_data ) unsigned int ret = 0; for( int ix = 0; ix < 5; ++ix ) { char * s = strchr( encodeTable, fiveChars[ ix ] ); -if( s == 0 ) BOOST_THROW_EXCEPTION(bad_input_data()); +if( s == 0 ) LLTHROW(bad_input_data()); ret = ret * 85 + (s-encodeTable); } return ret; diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index f51279e817..8bace8ac41 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -237,8 +237,21 @@ namespace tut void ErrorTestObject::test<4>() // file abbreviation { - std::string thisFile = __FILE__; - std::string abbreviateFile = LLError::abbreviateFile(thisFile); + std::string prev, abbreviateFile = __FILE__; + do + { + prev = abbreviateFile; + abbreviateFile = LLError::abbreviateFile(abbreviateFile); + // __FILE__ is assumed to end with + // indra/llcommon/tests/llerror_test.cpp. This test used to call + // abbreviateFile() exactly once, then check below whether it + // still contained the string 'indra'. That fails if the FIRST + // part of the pathname also contains indra! Certain developer + // machine images put local directory trees under + // /ngi-persist/indra, which is where we observe the problem. So + // now, keep calling abbreviateFile() until it returns its + // argument unchanged, THEN check. + } while (abbreviateFile != prev); ensure_ends_with("file name abbreviation", abbreviateFile, diff --git a/indra/llcommon/tests/llexception_test.cpp b/indra/llcommon/tests/llexception_test.cpp new file mode 100644 index 0000000000..6bee1943c2 --- /dev/null +++ b/indra/llcommon/tests/llexception_test.cpp @@ -0,0 +1,308 @@ +/** + * @file llexception_test.cpp + * @author Nat Goodspeed + * @date 2016-08-12 + * @brief Tests for throwing exceptions. + * + * This isn't a regression test: it doesn't need to be run every build, which + * is why the corresponding line in llcommon/CMakeLists.txt is commented out. + * Rather it's a head-to-head test of what kind of exception information we + * can collect from various combinations of exception base classes, type of + * throw verb and sequences of catch clauses. + * + * This "test" makes no ensure() calls: its output goes to stdout for human + * examination. + * + * As of 2016-08-12 with Boost 1.57, we come to the following conclusions. + * These should probably be re-examined from time to time as we update Boost. + * + * - It is indisputably beneficial to use BOOST_THROW_EXCEPTION() rather than + * plain throw. The macro annotates the exception object with the filename, + * line number and function name from which the exception was thrown. + * + * - That being the case, deriving only from boost::exception isn't an option. + * Every exception object passed to BOOST_THROW_EXCEPTION() must be derived + * directly or indirectly from std::exception. The only question is whether + * to also derive from boost::exception. We decided to derive LLException + * from both, as it makes message output slightly cleaner, but this is a + * trivial reason: if a strong reason emerges to prefer single inheritance, + * dropping the boost::exception base class shouldn't be a problem. + * + * - (As you will have guessed, ridiculous things like a char* or int or a + * class derived from neither boost::exception nor std::exception can only + * be caught by that specific type or (...), and + * boost::current_exception_diagnostic_information() simply throws up its + * hands and confesses utter ignorance. Stay away from such nonsense.) + * + * - But if you derive from std::exception, to nat's surprise, + * boost::current_exception_diagnostic_information() gives as much + * information about exceptions in a catch (...) clause as you can get from + * a specific catch (const std::exception&) clause, notably the concrete + * exception class and the what() string. So instead of a sequence like + * + * try { ... } + * catch (const boost::exception& e) { ... boost-flavored logging ... } + * catch (const std::exception& e) { ... std::exception logging ... } + * catch (...) { ... generic logging ... } + * + * we should be able to get away with only a catch (...) clause that logs + * boost::current_exception_diagnostic_information(). + * + * - Going further: boost::current_exception_diagnostic_information() provides + * just as much information even within a std::set_terminate() handler. So + * it might not even be strictly necessary to include a catch (...) clause + * since the viewer does use std::set_terminate(). + * + * - (We might consider adding a catch (int) clause because Kakadu internally + * throws ints, and who knows if one of those might leak out. If it does, + * boost::current_exception_diagnostic_information() can do nothing with it. + * A catch (int) clause could at least log the value and rethrow.) + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llexception.h" +// STL headers +// std headers +#include <typeinfo> +// external library headers +#include <boost/throw_exception.hpp> +// other Linden headers +#include "../test/lltut.h" + +// helper for display output +// usage: std::cout << center(some string value, fill char, width) << std::endl; +// (assumes it's the only thing on that particular line) +struct center +{ + center(const std::string& label, char fill, std::size_t width): + mLabel(label), + mFill(fill), + mWidth(width) + {} + + // Use friend declaration not because we need to grant access, but because + // it lets us declare a free operator like a member function. + friend std::ostream& operator<<(std::ostream& out, const center& ctr) + { + std::size_t padded = ctr.mLabel.length() + 2; + std::size_t left = (ctr.mWidth - padded) / 2; + std::size_t right = ctr.mWidth - left - padded; + return out << std::string(left, ctr.mFill) << ' ' << ctr.mLabel << ' ' + << std::string(right, ctr.mFill); + } + + std::string mLabel; + char mFill; + std::size_t mWidth; +}; + +/***************************************************************************** +* Four kinds of exceptions: derived from boost::exception, from +* std::exception, from both, from neither +*****************************************************************************/ +// Interestingly, we can't use this variant with BOOST_THROW_EXCEPTION() +// (which we want) -- we reach a failure topped by this comment: +// //All boost exceptions are required to derive from std::exception, +// //to ensure compatibility with BOOST_NO_EXCEPTIONS. +struct FromBoost: public boost::exception +{ + FromBoost(const std::string& what): mWhat(what) {} + ~FromBoost() throw() {} + std::string what() const { return mWhat; } + std::string mWhat; +}; + +struct FromStd: public std::runtime_error +{ + FromStd(const std::string& what): std::runtime_error(what) {} +}; + +struct FromBoth: public boost::exception, public std::runtime_error +{ + FromBoth(const std::string& what): std::runtime_error(what) {} +}; + +// Same deal with FromNeither: can't use with BOOST_THROW_EXCEPTION(). +struct FromNeither +{ + FromNeither(const std::string& what): mWhat(what) {} + std::string what() const { return mWhat; } + std::string mWhat; +}; + +/***************************************************************************** +* Two kinds of throws: plain throw and BOOST_THROW_EXCEPTION() +*****************************************************************************/ +template <typename EXC> +void plain_throw(const std::string& what) +{ + throw EXC(what); +} + +template <typename EXC> +void boost_throw(const std::string& what) +{ + BOOST_THROW_EXCEPTION(EXC(what)); +} + +// Okay, for completeness, functions that throw non-class values. We wouldn't +// even deign to consider these if we hadn't found examples in our own source +// code! (Note that Kakadu's internal exception support is still based on +// throwing ints.) +void throw_char_ptr(const std::string& what) +{ + throw what.c_str(); // umm... +} + +void throw_int(const std::string& what) +{ + throw int(what.length()); +} + +/***************************************************************************** +* Three sequences of catch clauses: +* boost::exception then ..., +* std::exception then ..., +* or just ... +*****************************************************************************/ +void catch_boost_dotdotdot(void (*thrower)(const std::string&), const std::string& what) +{ + try + { + thrower(what); + } + catch (const boost::exception& e) + { + std::cout << "catch (const boost::exception& e)" << std::endl; + std::cout << "e is " << typeid(e).name() << std::endl; + std::cout << "boost::diagnostic_information(e):\n'" + << boost::diagnostic_information(e) << "'" << std::endl; + // no way to report e.what() + } + catch (...) + { + std::cout << "catch (...)" << std::endl; + std::cout << "boost::current_exception_diagnostic_information():\n'" + << boost::current_exception_diagnostic_information() << "'" + << std::endl; + } +} + +void catch_std_dotdotdot(void (*thrower)(const std::string&), const std::string& what) +{ + try + { + thrower(what); + } + catch (const std::exception& e) + { + std::cout << "catch (const std::exception& e)" << std::endl; + std::cout << "e is " << typeid(e).name() << std::endl; + std::cout << "boost::diagnostic_information(e):\n'" + << boost::diagnostic_information(e) << "'" << std::endl; + std::cout << "e.what: '" + << e.what() << "'" << std::endl; + } + catch (...) + { + std::cout << "catch (...)" << std::endl; + std::cout << "boost::current_exception_diagnostic_information():\n'" + << boost::current_exception_diagnostic_information() << "'" + << std::endl; + } +} + +void catch_dotdotdot(void (*thrower)(const std::string&), const std::string& what) +{ + try + { + thrower(what); + } + catch (...) + { + std::cout << "catch (...)" << std::endl; + std::cout << "boost::current_exception_diagnostic_information():\n'" + << boost::current_exception_diagnostic_information() << "'" + << std::endl; + } +} + +/***************************************************************************** +* Try a particular kind of throw against each of three catch sequences +*****************************************************************************/ +void catch_several(void (*thrower)(const std::string&), const std::string& what) +{ + std::cout << std::string(20, '-') << "catch_boost_dotdotdot(" << what << ")" << std::endl; + catch_boost_dotdotdot(thrower, "catch_boost_dotdotdot(" + what + ")"); + + std::cout << std::string(20, '-') << "catch_std_dotdotdot(" << what << ")" << std::endl; + catch_std_dotdotdot(thrower, "catch_std_dotdotdot(" + what + ")"); + + std::cout << std::string(20, '-') << "catch_dotdotdot(" << what << ")" << std::endl; + catch_dotdotdot(thrower, "catch_dotdotdot(" + what + ")"); +} + +/***************************************************************************** +* For a particular kind of exception, try both kinds of throw against all +* three catch sequences +*****************************************************************************/ +template <typename EXC> +void catch_both_several(const std::string& what) +{ + std::cout << std::string(20, '*') << "plain_throw<" << what << ">" << std::endl; + catch_several(plain_throw<EXC>, "plain_throw<" + what + ">"); + + std::cout << std::string(20, '*') << "boost_throw<" << what << ">" << std::endl; + catch_several(boost_throw<EXC>, "boost_throw<" + what + ">"); +} + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llexception_data + { + }; + typedef test_group<llexception_data> llexception_group; + typedef llexception_group::object object; + llexception_group llexceptiongrp("llexception"); + + template<> template<> + void object::test<1>() + { + set_test_name("throwing exceptions"); + + // For each kind of exception, try both kinds of throw against all + // three catch sequences + std::size_t margin = 72; + std::cout << center("FromStd", '=', margin) << std::endl; + catch_both_several<FromStd>("FromStd"); + + std::cout << center("FromBoth", '=', margin) << std::endl; + catch_both_several<FromBoth>("FromBoth"); + + std::cout << center("FromBoost", '=', margin) << std::endl; + // can't throw with BOOST_THROW_EXCEPTION(), just use catch_several() + catch_several(plain_throw<FromBoost>, "plain_throw<FromBoost>"); + + std::cout << center("FromNeither", '=', margin) << std::endl; + // can't throw this with BOOST_THROW_EXCEPTION() either + catch_several(plain_throw<FromNeither>, "plain_throw<FromNeither>"); + + std::cout << center("const char*", '=', margin) << std::endl; + // We don't expect BOOST_THROW_EXCEPTION() to throw anything so daft + // as a const char* or an int, so don't bother with + // catch_both_several() -- just catch_several(). + catch_several(throw_char_ptr, "throw_char_ptr"); + + std::cout << center("int", '=', margin) << std::endl; + catch_several(throw_int, "throw_int"); + } +} // namespace tut diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index fa16fd6915..9a4bbbd630 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -40,7 +40,6 @@ #include <boost/bind.hpp> #include <boost/noncopyable.hpp> #include <boost/shared_ptr.hpp> -#include <boost/throw_exception.hpp> #include <list> #include <string> @@ -79,7 +78,7 @@ struct WrapLLErrs error = message; // Also throw an appropriate exception since calling code is likely to // assume that control won't continue beyond LL_ERRS. - BOOST_THROW_EXCEPTION(FatalException(message)); + LLTHROW(FatalException(message)); } std::string error; |