From c82995f3e67ed1947c29fa1d27f1dfe015513ab0 Mon Sep 17 00:00:00 2001 From: Mnikolenko Productengine Date: Tue, 7 Sep 2021 18:46:40 +0300 Subject: SL-15832 Add OS bitness to ViewerStats --- indra/llcommon/llsys.cpp | 26 ++++++++++++++++++++++++++ indra/llcommon/llsys.h | 4 ++++ 2 files changed, 30 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index 4e61fb8a58..b3a93e3254 100644 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -474,6 +474,8 @@ LLOSInfo::LLOSInfo() : dotted_version_string << mMajorVer << "." << mMinorVer << "." << mBuild; mOSVersionString.append(dotted_version_string.str()); + mOSBitness = is64Bit() ? 64 : 32; + LL_INFOS("LLOSInfo") << "OS bitness: " << mOSBitness << LL_ENDL; } #ifndef LL_WINDOWS @@ -529,6 +531,11 @@ const std::string& LLOSInfo::getOSVersionString() const return mOSVersionString; } +const S32 LLOSInfo::getOSBitness() const +{ + return mOSBitness; +} + //static U32 LLOSInfo::getProcessVirtualSizeKB() { @@ -582,6 +589,25 @@ U32 LLOSInfo::getProcessResidentSizeKB() return resident_size; } +//static +bool LLOSInfo::is64Bit() +{ +#if LL_WINDOWS +#if defined(_WIN64) + return true; +#elif defined(_WIN32) + // 32-bit viewer may be run on both 32-bit and 64-bit Windows, need to elaborate + BOOL f64 = FALSE; + return IsWow64Process(GetCurrentProcess(), &f64) && f64; +#else + return false; +#endif +#else // ! LL_WINDOWS + // we only build a 64-bit mac viewer and currently we don't build for linux at all + return true; +#endif +} + LLCPUInfo::LLCPUInfo() { std::ostringstream out; diff --git a/indra/llcommon/llsys.h b/indra/llcommon/llsys.h index 5ab97939b9..cb92cb0ac6 100644 --- a/indra/llcommon/llsys.h +++ b/indra/llcommon/llsys.h @@ -51,6 +51,8 @@ public: const std::string& getOSStringSimple() const; const std::string& getOSVersionString() const; + + const S32 getOSBitness() const; S32 mMajorVer; S32 mMinorVer; @@ -59,6 +61,7 @@ public: #ifndef LL_WINDOWS static S32 getMaxOpenFiles(); #endif + static bool is64Bit(); static U32 getProcessVirtualSizeKB(); static U32 getProcessResidentSizeKB(); @@ -66,6 +69,7 @@ private: std::string mOSString; std::string mOSStringSimple; std::string mOSVersionString; + S32 mOSBitness; }; -- cgit v1.2.3 From 6e200800eddc374419d6e0e42b9ef63813ddf70f Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Wed, 17 Mar 2021 00:19:38 +0200 Subject: SL-14541 removed breakpad, win_crash_logger # Conflicts: # autobuild.xml # indra/CMakeLists.txt # indra/newview/CMakeLists.txt # indra/newview/llappviewerwin32.h # indra/win_crash_logger/llcrashloggerwindows.cpp Cherry picked from DRTVWR-520 --- indra/llcommon/CMakeLists.txt | 3 - indra/llcommon/llapp.cpp | 173 ------------------------------------------ indra/llcommon/llapp.h | 8 -- 3 files changed, 184 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index dd266630ea..22dfe12e40 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -9,7 +9,6 @@ include(Linking) include(Boost) include(LLSharedLibs) include(JsonCpp) -include(GoogleBreakpad) include(Copy3rdPartyLibs) include(ZLIB) include(URIPARSER) @@ -19,7 +18,6 @@ include_directories( ${LLCOMMON_INCLUDE_DIRS} ${JSONCPP_INCLUDE_DIR} ${ZLIB_INCLUDE_DIRS} - ${BREAKPAD_INCLUDE_DIRECTORIES} ${URIPARSER_INCLUDE_DIRS} ) @@ -288,7 +286,6 @@ endif(LLCOMMON_LINK_SHARED) target_link_libraries( llcommon - ${BREAKPAD_EXCEPTION_HANDLER_LIBRARIES} ${APRUTIL_LIBRARIES} ${APR_LIBRARIES} ${EXPAT_LIBRARIES} diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 6064a843ae..a5300adf87 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -46,7 +46,6 @@ #include "llstl.h" // for DeletePointer() #include "llstring.h" #include "lleventtimer.h" -#include "google_breakpad/exception_handler.h" #include "stringize.h" #include "llcleanup.h" #include "llevents.h" @@ -62,12 +61,6 @@ LONG WINAPI default_windows_exception_handler(struct _EXCEPTION_POINTERS *exception_infop); BOOL ConsoleCtrlHandler(DWORD fdwCtrlType); -bool windows_post_minidump_callback(const wchar_t* dump_path, - const wchar_t* minidump_id, - void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion, - bool succeeded); #else # include # include // for fork() @@ -146,8 +139,6 @@ void LLApp::commonCtor() // Set the application to this instance. sApplication = this; - - mExceptionHandler = 0; // initialize the buffer to write the minidump filename to // (this is used to avoid allocating memory in the crash handler) @@ -177,8 +168,6 @@ LLApp::~LLApp() delete mThreadErrorp; mThreadErrorp = NULL; } - - if(mExceptionHandler != 0) delete mExceptionHandler; SUBSYSTEM_CLEANUP_DBG(LLCommon); } @@ -394,69 +383,6 @@ void LLApp::setupErrorHandling(bool second_instance) #if LL_WINDOWS -#if LL_SEND_CRASH_REPORTS && ! defined(LL_BUGSPLAT) - EnableCrashingOnCrashes(); - - // This sets a callback to handle w32 signals to the console window. - // The viewer shouldn't be affected, sicne its a windowed app. - SetConsoleCtrlHandler( (PHANDLER_ROUTINE) ConsoleCtrlHandler, TRUE); - - // Install the Google Breakpad crash handler for Windows - if(mExceptionHandler == 0) - { - if ( second_instance ) //BUG-5707 Firing teleport from a web browser causes second - { - mExceptionHandler = new google_breakpad::ExceptionHandler( - L"C:\\Temp\\", - 0, //No filter - windows_post_minidump_callback, - 0, - google_breakpad::ExceptionHandler::HANDLER_ALL); //No custom client info. - } - else - { - LL_WARNS() << "adding breakpad exception handler" << LL_ENDL; - - std::wstring wpipe_name; - wpipe_name = mCrashReportPipeStr + wstringize(getPid()); - - const std::wstring wdump_path(utf8str_to_utf16str(mDumpPath)); - - int retries = 30; - for (; retries > 0; --retries) - { - if (mExceptionHandler != 0) delete mExceptionHandler; - - mExceptionHandler = new google_breakpad::ExceptionHandler( - wdump_path, - NULL, //No filter - windows_post_minidump_callback, - 0, - google_breakpad::ExceptionHandler::HANDLER_ALL, - MiniDumpNormal, //Generate a 'normal' minidump. - wpipe_name.c_str(), - NULL); //No custom client info. - if (mExceptionHandler->IsOutOfProcess()) - { - LL_INFOS("CRASHREPORT") << "Successfully attached to Out of Process exception handler." << LL_ENDL; - break; - } - else - { - LL_WARNS("CRASHREPORT") << "Unable to attach to Out of Process exception handler. " << retries << " retries remaining." << LL_ENDL; - ::Sleep(100); //Wait a tick and try again. - } - } - - if (retries == 0) LL_WARNS("CRASHREPORT") << "Unable to attach to Out of Process exception handler." << LL_ENDL; - } - - if (mExceptionHandler) - { - mExceptionHandler->set_handle_debug_exceptions(true); - } - } -#endif // LL_SEND_CRASH_REPORTS && ! defined(LL_BUGSPLAT) #else // ! LL_WINDOWS #if defined(LL_BUGSPLAT) @@ -511,10 +437,6 @@ void LLApp::setupErrorHandling(bool second_instance) } #endif // ! LL_RELEASE_FOR_DOWNLOAD - if(installHandler && (mExceptionHandler == 0)) - { - mExceptionHandler = new google_breakpad::ExceptionHandler(mDumpPath, 0, &unix_post_minidump_callback, 0, true, 0); - } #elif LL_LINUX if(installHandler && (mExceptionHandler == 0)) { @@ -614,31 +536,6 @@ void LLApp::setError() setStatus(APP_STATUS_ERROR); } -void LLApp::setMiniDumpDir(const std::string &path) -{ - if (path.empty()) - { - mDumpPath = "/tmp"; - } - else - { - mDumpPath = path; - } - - if(mExceptionHandler == 0) return; -#ifdef LL_WINDOWS - std::wstring buffer(utf8str_to_utf16str(mDumpPath)); - if (buffer.size() > MAX_MINDUMP_PATH_LENGTH) buffer.resize(MAX_MINDUMP_PATH_LENGTH); - mExceptionHandler->set_dump_path(buffer); -#elif LL_LINUX - //google_breakpad::MinidumpDescriptor desc("/tmp"); //path works in debug fails in production inside breakpad lib so linux gets a little less stack reporting until it is patched. - google_breakpad::MinidumpDescriptor desc(mDumpPath); //path works in debug fails in production inside breakpad lib so linux gets a little less stack reporting until it is patched. - mExceptionHandler->set_minidump_descriptor(desc); -#else - mExceptionHandler->set_dump_path(mDumpPath); -#endif -} - void LLApp::setDebugFileNames(const std::string &path) { mStaticDebugFileName = path + "static_debug_info.log"; @@ -647,8 +544,6 @@ void LLApp::setDebugFileNames(const std::string &path) void LLApp::writeMiniDump() { - if(mExceptionHandler == 0) return; - mExceptionHandler->WriteMinidump(); } // static @@ -705,13 +600,6 @@ bool LLApp::isExiting() void LLApp::disableCrashlogger() { - // Disable Breakpad exception handler. - if (mExceptionHandler != 0) - { - delete mExceptionHandler; - mExceptionHandler = 0; - } - sDisableCrashlogger = TRUE; } @@ -1105,64 +993,3 @@ bool unix_post_minidump_callback(const char *dump_dir, } #endif // !WINDOWS -#ifdef LL_WINDOWS -bool windows_post_minidump_callback(const wchar_t* dump_path, - const wchar_t* minidump_id, - void* context, - EXCEPTION_POINTERS* exinfo, - MDRawAssertionInfo* assertion, - bool succeeded) -{ - char * path = LLApp::instance()->getMiniDumpFilename(); - S32 remaining = LLApp::MAX_MINDUMP_PATH_LENGTH; - size_t bytesUsed; - - LL_INFOS("MINIDUMPCALLBACK") << "Dump file was generated." << LL_ENDL; - bytesUsed = wcstombs(path, dump_path, static_cast(remaining)); - remaining -= bytesUsed; - path += bytesUsed; - if(remaining > 0 && bytesUsed > 0 && path[-1] != '\\') - { - *path++ = '\\'; - --remaining; - } - if(remaining > 0) - { - bytesUsed = wcstombs(path, minidump_id, static_cast(remaining)); - remaining -= bytesUsed; - path += bytesUsed; - } - if(remaining > 0) - { - strncpy(path, ".dmp", remaining); - } - - LL_INFOS("CRASHREPORT") << "generated minidump: " << LLApp::instance()->getMiniDumpFilename() << LL_ENDL; - // *NOTE:Mani - this code is stolen from LLApp, where its never actually used. - //OSMessageBox("Attach Debugger Now", "Error", OSMB_OK); - // *TODO: Translate the signals/exceptions into cross-platform stuff - // Windows implementation - LL_INFOS() << "Entering Windows Exception Handler..." << LL_ENDL; - - if (LLApp::isError()) - { - LL_WARNS() << "Got another fatal signal while in the error handler, die now!" << LL_ENDL; - } - - // Flag status to error, so thread_error starts its work - LLApp::setError(); - - // Block in the exception handler until the app has stopped - // This is pretty sketchy, but appears to work just fine - while (!LLApp::isStopped()) - { - ms_sleep(10); - } - -#ifndef LL_RELEASE_FOR_DOWNLOAD - return false; -#else - return true; -#endif -} -#endif diff --git a/indra/llcommon/llapp.h b/indra/llcommon/llapp.h index 5fa91b8bf5..83f3bf3f93 100644 --- a/indra/llcommon/llapp.h +++ b/indra/llcommon/llapp.h @@ -49,10 +49,6 @@ void clear_signals(); #endif -namespace google_breakpad { - class ExceptionHandler; // See exception_handler.h -} - class LL_COMMON_API LLApp { friend class LLErrorThread; @@ -236,7 +232,6 @@ public: static const U32 MAX_MINDUMP_PATH_LENGTH = 256; // change the directory where Breakpad minidump files are written to - void setMiniDumpDir(const std::string &path); void setDebugFileNames(const std::string &path); // Return the Google Breakpad minidump filename after a crash. @@ -316,9 +311,6 @@ private: private: // the static application instance if it was created. static LLApp* sApplication; - - google_breakpad::ExceptionHandler * mExceptionHandler; - #if !LL_WINDOWS friend void default_unix_signal_handler(int signum, siginfo_t *info, void *); -- cgit v1.2.3 From 5af8f15a0579bc88ca4249324db8b1f19c52bbd5 Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Wed, 8 Sep 2021 20:53:50 +0300 Subject: SL-14541 Replace zlib with zlib-ng --- indra/llcommon/CMakeLists.txt | 6 +++--- indra/llcommon/llsdserialize.cpp | 2 +- indra/llcommon/llsys.cpp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 22dfe12e40..040b0eb23d 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -10,14 +10,14 @@ include(Boost) include(LLSharedLibs) include(JsonCpp) include(Copy3rdPartyLibs) -include(ZLIB) +include(ZLIBNG) include(URIPARSER) include_directories( ${EXPAT_INCLUDE_DIRS} ${LLCOMMON_INCLUDE_DIRS} ${JSONCPP_INCLUDE_DIR} - ${ZLIB_INCLUDE_DIRS} + ${ZLIBNG_INCLUDE_DIRS} ${URIPARSER_INCLUDE_DIRS} ) @@ -290,7 +290,7 @@ target_link_libraries( ${APR_LIBRARIES} ${EXPAT_LIBRARIES} ${JSONCPP_LIBRARIES} - ${ZLIB_LIBRARIES} + ${ZLIBNG_LIBRARIES} ${WINDOWS_LIBRARIES} ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 022a5d4659..8b4a0ee6d8 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -37,7 +37,7 @@ #ifdef LL_USESYSTEMLIBS # include #else -# include "zlib/zlib.h" // for davep's dirty little zip functions +# include "zlib-ng/zlib.h" // for davep's dirty little zip functions #endif #if !LL_WINDOWS diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index b3a93e3254..f710f1b279 100644 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -36,7 +36,7 @@ #ifdef LL_USESYSTEMLIBS # include #else -# include "zlib/zlib.h" +# include "zlib-ng/zlib.h" #endif #include "llprocessor.h" -- cgit v1.2.3 From a791a22b1b00416fc6f84240dd95113a3787668d Mon Sep 17 00:00:00 2001 From: Andrey Kleshchev Date: Thu, 9 Sep 2021 00:20:57 +0300 Subject: SL-14541 remove mac-crash-logger --- indra/llcommon/llapp.cpp | 70 ++++++------------------------------------------ 1 file changed, 8 insertions(+), 62 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index a5300adf87..df2a066f62 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -385,70 +385,16 @@ void LLApp::setupErrorHandling(bool second_instance) #else // ! LL_WINDOWS -#if defined(LL_BUGSPLAT) - // Don't install our own signal handlers -- BugSplat needs to hook them, - // or it's completely ineffectual. - bool installHandler = false; - -#else // ! LL_BUGSPLAT - // - // Start up signal handling. - // - // There are two different classes of signals. Synchronous signals are delivered to a specific - // thread, asynchronous signals can be delivered to any thread (in theory) - // - setup_signals(); - - // Add google breakpad exception handler configured for Darwin/Linux. - bool installHandler = true; +#if ! defined(LL_BUGSPLAT) + // + // Start up signal handling. + // + // There are two different classes of signals. Synchronous signals are delivered to a specific + // thread, asynchronous signals can be delivered to any thread (in theory) + // + setup_signals(); #endif // ! LL_BUGSPLAT -#if LL_DARWIN - // For the special case of Darwin, we do not want to install the handler if - // the process is being debugged as the app will exit with value ABRT (6) if - // we do. Unfortunately, the code below which performs that test relies on - // the structure kinfo_proc which has been tagged by apple as an unstable - // API. We disable this test for shipping versions to avoid conflicts with - // future releases of Darwin. This test is really only needed for developers - // starting the app from a debugger anyway. - #ifndef LL_RELEASE_FOR_DOWNLOAD - int mib[4]; - mib[0] = CTL_KERN; - mib[1] = KERN_PROC; - mib[2] = KERN_PROC_PID; - mib[3] = getpid(); - - struct kinfo_proc info; - memset(&info, 0, sizeof(info)); - - size_t size = sizeof(info); - int result = sysctl(mib, sizeof(mib) / sizeof(*mib), &info, &size, NULL, 0); - if((result == 0) || (errno == ENOMEM)) - { - // P_TRACED flag is set, so this process is being debugged; do not install - // the handler - if(info.kp_proc.p_flag & P_TRACED) installHandler = false; - } - else - { - // Failed to discover if the process is being debugged; default to - // installing the handler. - installHandler = true; - } - #endif // ! LL_RELEASE_FOR_DOWNLOAD - -#elif LL_LINUX - if(installHandler && (mExceptionHandler == 0)) - { - if (mDumpPath.empty()) - { - mDumpPath = "/tmp"; - } - google_breakpad::MinidumpDescriptor desc(mDumpPath); - mExceptionHandler = new google_breakpad::ExceptionHandler(desc, NULL, unix_minidump_callback, NULL, true, -1); - } -#endif // LL_LINUX - #endif // ! LL_WINDOWS #ifdef LL_BUGSPLAT -- cgit v1.2.3 From 2b96d1bbe00c317ea8dfe420dd4167dde5d153ae Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Sep 2021 16:25:12 -0400 Subject: DRTVWR-543: Add ClassicCallback utility class with tests --- indra/llcommon/CMakeLists.txt | 8 +- indra/llcommon/classic_callback.cpp | 16 ++ indra/llcommon/classic_callback.h | 292 +++++++++++++++++++++++++ indra/llcommon/tests/classic_callback_test.cpp | 150 +++++++++++++ 4 files changed, 463 insertions(+), 3 deletions(-) create mode 100644 indra/llcommon/classic_callback.cpp create mode 100644 indra/llcommon/classic_callback.h create mode 100644 indra/llcommon/tests/classic_callback_test.cpp (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index dd266630ea..2d17d1b09a 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -127,6 +127,7 @@ set(llcommon_SOURCE_FILES set(llcommon_HEADER_FILES CMakeLists.txt + classic_callback.h ctype_workaround.h fix_macros.h indra_constants.h @@ -328,16 +329,17 @@ if (LL_TESTS) ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY}) - LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}") LL_ADD_INTEGRATION_TEST(bitpack "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(classic_callback "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llbase64 "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llcond "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lldate "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lldeadmantimer "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lldependencies "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llerror "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(lleventdispatcher "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lleventcoro "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lleventdispatcher "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lleventfilter "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llframetimer "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llheteromap "" "${test_libs}") @@ -355,8 +357,8 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llstring "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltrace "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltreeiterators "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(lluri "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llunits "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lluri "" "${test_libs}") LL_ADD_INTEGRATION_TEST(stringize "" "${test_libs}") ## llexception_test.cpp isn't a regression test, and doesn't need to be run diff --git a/indra/llcommon/classic_callback.cpp b/indra/llcommon/classic_callback.cpp new file mode 100644 index 0000000000..5674e0a44d --- /dev/null +++ b/indra/llcommon/classic_callback.cpp @@ -0,0 +1,16 @@ +/** + * @file classic_callback.cpp + * @author Nat Goodspeed + * @date 2021-09-23 + * @brief Implementation for classic_callback. + * + * $LicenseInfo:firstyear=2021&license=viewerlgpl$ + * Copyright (c) 2021, Linden Research, Inc. + * $/LicenseInfo$ + */ + +namespace { + +const char dummy[] = "cpp file required to build test program"; + +} // anonymous namespace diff --git a/indra/llcommon/classic_callback.h b/indra/llcommon/classic_callback.h new file mode 100644 index 0000000000..76ff9c2339 --- /dev/null +++ b/indra/llcommon/classic_callback.h @@ -0,0 +1,292 @@ +/** + * @file classic_callback.h + * @author Nat Goodspeed + * @date 2016-06-21 + * @brief ClassicCallback and HeapClassicCallback + * + * This header file addresses the problem of passing a method on a C++ object + * to an API that requires a classic-C function pointer. Typically such a + * callback API accepts a void* pointer along with the function pointer, and + * the function pointer signature accepts a void* parameter. The API passes + * the caller's pointer value into the callback function so it can find its + * data. In C++, there are a few ways to deal with this case: + * + * - Use a static method with correct signature. If you don't need access to a + * specific instance, that works fine. + * - Store the object statically (or store a static pointer to a non-static + * instance). As long as you only care about one instance, that works, but + * starts to get a little icky. As soon as there's more than one pertinent + * instance, fight valiantly against the temptation to stuff the instance + * pointer into a static pointer variable "just for a moment." + * - Code a static trampoline callback function that accepts the void* user + * data pointer, casts it to the appropriate class type and calls the actual + * method on that class. + * + * ClassicCallback encapsulates the last. You need only construct a + * ClassicCallback instance somewhere that will survive until the callback is + * called, binding the target C++ callable. You then call its get_callback() + * and get_userdata() methods to pass an appropriate classic-C function + * pointer and void* user data pointer, respectively, to the old-style + * callback API. get_callback() synthesizes a static trampoline function + * that casts the user data pointer and calls the bound C++ callable. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_CLASSIC_CALLBACK_H) +#define LL_CLASSIC_CALLBACK_H + +#include +#include // std::is_same + +/***************************************************************************** +* Helpers +*****************************************************************************/ + +// find a type in a parameter pack: http://stackoverflow.com/q/17844867/5533635 +// usage: index_of<0, sought_t, PackName...>::value +template +struct index_of +{ + static constexpr int const value = + std::is_same::value ? + idx : index_of::value; +}; + +// recursion tail +template +struct index_of +{ + static constexpr int const value = + std::is_same::value ? idx : -1; +}; + +/***************************************************************************** +* ClassicCallback +*****************************************************************************/ +/** + * Instantiate ClassicCallback in whatever storage will persist long enough + * for the callback to be called. It holds a modern C++ callable, providing a + * static function pointer and a USERDATA (default void*) capable of being + * passed through a classic-C callback API. When the static function is called + * with that USERDATA pointer, ClassicCallback forwards the call to the bound + * C++ callable. + * + * Usage: + * @code + * // callback signature required by the API of interest + * typedef void (*callback_t)(int, const char*, void*, double); + * // old-style API that accepts a classic-C callback function pointer + * void oldAPI(callback_t callback, void* userdata); + * // but I want to pass a lambda that references data local to my function! + * // (We don't need to name the void* parameter in the C++ callable; + * // ClassicCallback already used it to locate the lambda instance.) + * auto ccb{ + * makeClassicCallback( + * [=](int n, const char* s, void*, double f){ ... }) }; + * oldAPI(ccb.get_callback(), ccb.get_userdata()); + * // If the passed callback is called before oldAPI() returns, we can now + * // safely destroy ccb. If the callback might be called later, consider + * // HeapClassicCallback instead. + * @endcode + * + * If you have a callable object in hand, and you want to pass that to + * ClassicCallback, you may either consume it by passing std::move(object), or + * explicitly specify a reference to that object type as the CALLABLE template + * parameter: + * @code + * CallableObject obj; + * ClassicCallback ccb{obj}; + * @endcode + */ +// CALLABLE should either be deduced, e.g. by makeClassicCallback(), or +// specified explicitly. Its default type is meaningless, coded only so we can +// provide a useful default for USERDATA. +template +class ClassicCallback +{ + typedef ClassicCallback self_t; + +public: + /// ClassicCallback binds any modern C++ callable. + ClassicCallback(CALLABLE&& callable): + mCallable(std::forward(callable)) + {} + + /** + * ClassicCallback must not itself be copied or moved! Once you've passed + * get_userdata() to some API, this object MUST remain at that address. + */ + // However, we can't yet count on C++17 Class Template Argument Deduction, + // which means makeClassicCallback() is still useful, which means we MUST + // be able to return one to construct into caller's instance (move ctor). + // Possible defense: bool 'referenced' data member set by get_userdata(), + // with an llassert_always(! referenced) check in the move constructor. + ClassicCallback(ClassicCallback const&) = delete; + ClassicCallback(ClassicCallback&&) = default; // delete; + ClassicCallback& operator=(ClassicCallback const&) = delete; + ClassicCallback& operator=(ClassicCallback&&) = delete; + + /// Call get_callback() to get the necessary function pointer. + SIGNATURE get_callback() const + { + // This declaration is where the compiler instantiates the correct + // signature for the call() function template. + SIGNATURE callback = call; + return callback; + } + + /// Call get_userdata() to get the opaque USERDATA pointer to pass + /// through the classic-C callback API. + USERDATA get_userdata() const + { + // The USERDATA userdata is of course a pointer to this object. + return static_cast(const_cast(this)); + } + +protected: + /** + * This call() method accepts one or more callback arguments. It assumes + * the first USERDATA parameter is the userdata. + */ + // Note that we're not literally using C++ perfect forwarding here -- it + // doesn't work to specify (Args&&... args). But that's okay because we're + // dealing with a classic-C callback! It's not going to pass any move-only + // types. + template + static auto call(Args... args) + { + auto userdata = extract_userdata(std::forward(args)...); + // cast the userdata param to 'this' and call mCallable + return static_cast(userdata)-> + mCallable(std::forward(args)...); + } + + template + static USERDATA extract_userdata(Args... args) + { + // Search for the first void* parameter type, then extract that pointer. + // extract value from parameter pack: http://stackoverflow.com/a/24710433/5533635 + return std::get::value>(std::forward_as_tuple(args...)); + } + + CALLABLE mCallable; +}; + +/** + * Usage: + * @code + * auto ccb{ makeClassicCallback(actual_callback) }; + * @endcode + */ +template +auto makeClassicCallback(CALLABLE&& callable) +{ + return std::move(ClassicCallback + (std::forward(callable))); +} + +/***************************************************************************** +* HeapClassicCallback +*****************************************************************************/ +/** + * HeapClassicCallback is like ClassicCallback, with this exception: it MUST + * be allocated on the heap because, once the callback has been called, it + * deletes itself. This addresses the problem of a callback whose lifespan + * must persist beyond the scope in which the callback API is engaged -- but + * naturally this callback must be called exactly ONCE. + * + * Usage: + * @code + * // callback signature required by the API of interest + * typedef void (*callback_t)(int, const char*, void*, double); + * // here's the old-style API + * void oldAPI(callback_t callback, void* userdata); + * // want to call someObjPtr->method() when oldAPI() fires the callback, + * // sometime in the future after the enclosing function has returned + * auto ccb{ + * makeHeapClassicCallback( + * [someObjPtr](int n, const char* s, void*, double f) + * { someObjPtr->method(); }) }; + * oldAPI(ccb.get_callback(), ccb.get_userdata()); + * // We don't need a smart pointer for ccb, because it will be deleted once + * // oldAPI() calls the bound lambda. HeapClassicCallback is for when the + * // callback will be called exactly once. If the classic API might call the + * // passed callback more than once -- or might never call it at all -- + * // manually construct a ClassicCallback on the heap and manage its lifespan + * // explicitly. + * @endcode + */ +template +class HeapClassicCallback: public ClassicCallback +{ + typedef ClassicCallback super; + typedef HeapClassicCallback self_t; + + // This destructor is intentionally private to prevent allocation anywhere + // but the heap. (The Design and Evolution of C++, section 11.4.2: Control + // of Allocation) + ~HeapClassicCallback() {} + +public: + HeapClassicCallback(CALLABLE&& callable): + super(std::forward(callable)) + {} + + // makeHeapClassicCallback() only needs to return a pointer -- not an + // instance -- so we can lock down our move constructor too. + HeapClassicCallback(HeapClassicCallback&&) = delete; + + /// Replicate get_callback() from the base class because we must + /// instantiate OUR call() function template. + SIGNATURE get_callback() const + { + // This declaration is where the compiler instantiates the correct + // signature for the call() function template. + SIGNATURE callback = call; + return callback; + } + + /// Replicate get_userdata() from the base class because our call() + /// method must be able to reconstitute a pointer to this subclass. + USERDATA get_userdata() const + { + // The USERDATA userdata is of course a pointer to this object. + return static_cast(const_cast(this)); + } + +private: + // call() uses a helper class to delete the HeapClassicCallback when done, + // for two reasons. Most importantly, this deletes even if the callback + // throws an exception. But also, call() must directly return the callback + // result for return-type deduction. + struct Destroyer + { + Destroyer(self_t* p): mPtr(p) {} + ~Destroyer() { delete mPtr; } + + self_t* mPtr; + }; + + template + static auto call(Args... args) + { + // extract userdata at this level too + USERDATA userdata = super::extract_userdata(std::forward(args)...); + // arrange to delete it when we leave by whatever means + Destroyer destroy(static_cast(userdata)); + + return super::call(std::forward(args)...); + } +}; + +template +auto makeHeapClassicCallback(CALLABLE&& callable) +{ + return new HeapClassicCallback + (std::forward(callable)); +} + +#endif /* ! defined(LL_CLASSIC_CALLBACK_H) */ diff --git a/indra/llcommon/tests/classic_callback_test.cpp b/indra/llcommon/tests/classic_callback_test.cpp new file mode 100644 index 0000000000..232532b1c1 --- /dev/null +++ b/indra/llcommon/tests/classic_callback_test.cpp @@ -0,0 +1,150 @@ +/** + * @file classic_callback_test.cpp + * @author Nat Goodspeed + * @date 2021-09-22 + * @brief Test ClassicCallback and HeapClassicCallback. + * + * $LicenseInfo:firstyear=2021&license=viewerlgpl$ + * Copyright (c) 2021, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "classic_callback.h" +// STL headers +#include +#include +// std headers +// external library headers +// other Linden headers +#include "../test/lltut.h" + +/***************************************************************************** +* example callback accepting only (void* userdata) +*****************************************************************************/ +// callback_t is part of the specification of someAPI() +typedef void (*callback_t)(const char*, void*); +void someAPI(callback_t callback, void* userdata) +{ + callback("called", userdata); +} + +// C++ callable I want as the actual callback +struct MyCallback +{ + void operator()(const char* msg, void*) + { + mMsg = msg; + } + + void callback_with_extra(const std::string& extra, const char* msg) + { + mMsg = extra + ' ' + msg; + } + + std::string mMsg; +}; + +// a function for which I want to bind other data +void callback_with_extra(const std::string& extra, void*) +{ + std::cout << "callback_with_extra('" << extra << "', *)\n"; +} + +/***************************************************************************** +* example callback accepting several params, and void* userdata isn't first +*****************************************************************************/ +typedef std::string (*complex_callback)(int, const char*, void*, double); +std::string otherAPI(complex_callback callback, void* userdata) +{ + return callback(17, "hello world", userdata, 3.0); +} + +// struct into which we can capture complex_callback params +static struct Data +{ + void set(int i, const char* s, double f) + { + mi = i; + ms = s; + mf = f; + } + + void clear() { set(0, "", 0.0); } + + int mi; + std::string ms; + double mf; +} sData; + +// C++ callable I want to pass +struct OtherCallback +{ + std::string operator()(int num, const char* str, void*, double approx) + { + sData.set(num, str, approx); + return "hello back!"; + } +}; + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct classic_callback_data + { + }; + typedef test_group classic_callback_group; + typedef classic_callback_group::object object; + classic_callback_group classic_callbackgrp("classic_callback"); + + template<> template<> + void object::test<1>() + { + set_test_name("ClassicCallback"); + // engage someAPI(MyCallback()) + auto ccb{ makeClassicCallback(MyCallback()) }; + someAPI(ccb.get_callback(), ccb.get_userdata()); + // Unfortunately, with the side effect confined to the bound + // MyCallback instance, that call was invisible. Bind a reference to a + // named instance by specifying a ref type. + MyCallback mcb; + ClassicCallback ccb2(mcb); + someAPI(ccb2.get_callback(), ccb2.get_userdata()); + ensure_equals("failed to call through ClassicCallback", mcb.mMsg, "called"); + + // try with HeapClassicCallback + mcb.mMsg.clear(); + auto hcbp{ makeHeapClassicCallback(mcb) }; + someAPI(hcbp->get_callback(), hcbp->get_userdata()); + ensure_equals("failed to call through HeapClassicCallback", mcb.mMsg, "called"); + + // lambda + // The tricky thing here is that a lambda is an unspecified type, so + // you can't declare a ClassicCallback. + mcb.mMsg.clear(); + auto xcb( + makeClassicCallback( + [&mcb](const char* msg, void*) + { mcb.callback_with_extra("extra", msg); })); + someAPI(xcb.get_callback(), xcb.get_userdata()); + ensure_equals("failed to call lambda", mcb.mMsg, "extra called"); + + // engage otherAPI(OtherCallback()) + OtherCallback ocb; + // Instead of specifying a reference type for the bound CALLBACK, as + // with ccb2 above, you can alternatively move the callable object + // into the ClassicCallback (of course AFTER any other reference). + // That's why OtherCallback uses external data for its observable side + // effect. + auto occb{ makeClassicCallback(std::move(ocb)) }; + std::string result{ otherAPI(occb.get_callback(), occb.get_userdata()) }; + ensure_equals("failed to return callback result", result, "hello back!"); + ensure_equals("failed to set int", sData.mi, 17); + ensure_equals("failed to set string", sData.ms, "hello world"); + ensure_equals("failed to set double", sData.mf, 3.0); + } +} // namespace tut -- cgit v1.2.3 From 1c3d2876ac2b5212976bde9cd4fdea485a366ab9 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Sep 2021 17:09:16 -0400 Subject: DRTVWR-543: Consistently use ClassicCallback throughout. --- indra/llcommon/classic_callback.h | 4 ++-- indra/llcommon/tests/classic_callback_test.cpp | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/classic_callback.h b/indra/llcommon/classic_callback.h index 76ff9c2339..1ad6dbc58f 100644 --- a/indra/llcommon/classic_callback.h +++ b/indra/llcommon/classic_callback.h @@ -167,9 +167,9 @@ protected: template static USERDATA extract_userdata(Args... args) { - // Search for the first void* parameter type, then extract that pointer. + // Search for the first USERDATA parameter type, then extract that pointer. // extract value from parameter pack: http://stackoverflow.com/a/24710433/5533635 - return std::get::value>(std::forward_as_tuple(args...)); + return std::get::value>(std::forward_as_tuple(args...)); } CALLABLE mCallable; diff --git a/indra/llcommon/tests/classic_callback_test.cpp b/indra/llcommon/tests/classic_callback_test.cpp index 232532b1c1..c060775c24 100644 --- a/indra/llcommon/tests/classic_callback_test.cpp +++ b/indra/llcommon/tests/classic_callback_test.cpp @@ -22,7 +22,7 @@ #include "../test/lltut.h" /***************************************************************************** -* example callback accepting only (void* userdata) +* example callback *****************************************************************************/ // callback_t is part of the specification of someAPI() typedef void (*callback_t)(const char*, void*); @@ -47,12 +47,6 @@ struct MyCallback std::string mMsg; }; -// a function for which I want to bind other data -void callback_with_extra(const std::string& extra, void*) -{ - std::cout << "callback_with_extra('" << extra << "', *)\n"; -} - /***************************************************************************** * example callback accepting several params, and void* userdata isn't first *****************************************************************************/ -- cgit v1.2.3