From 9dd7c67012e305fa61d20135e6082389e622c88a Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Wed, 19 Apr 2017 16:50:56 -0700 Subject: Pull in improvements to LLProcess termination via a commit from Nat Linden here: https://bitbucket.org/rider_linden/doduo-viewer/commits/4f39500cb46e879dbb732e6547cc66f3ba39959e?at=default --- indra/llcommon/llprocess.cpp | 12 +++-- indra/llcommon/llprocess.h | 29 +++++++++-- indra/llcommon/tests/llprocess_test.cpp | 89 ++++++++++++++++++++++++++++----- 3 files changed, 110 insertions(+), 20 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp index 8c321d06b9..5753efdc59 100644 --- a/indra/llcommon/llprocess.cpp +++ b/indra/llcommon/llprocess.cpp @@ -517,6 +517,10 @@ LLProcessPtr LLProcess::create(const LLSDOrParams& params) LLProcess::LLProcess(const LLSDOrParams& params): mAutokill(params.autokill), + // Because 'autokill' originally meant both 'autokill' and 'attached', to + // preserve existing semantics, we promise that mAttached defaults to the + // same setting as mAutokill. + mAttached(params.attached.isProvided()? params.attached : params.autokill), mPipes(NSLOTS) { // Hmm, when you construct a ptr_vector with a size, it merely reserves @@ -625,9 +629,9 @@ LLProcess::LLProcess(const LLSDOrParams& params): // std handles and the like, and that's a bit more detachment than we // want. autokill=false just means not to implicitly kill the child when // the parent terminates! -// chkapr(apr_procattr_detach_set(procattr, params.autokill? 0 : 1)); +// chkapr(apr_procattr_detach_set(procattr, mAutokill? 0 : 1)); - if (params.autokill) + if (mAutokill) { #if ! defined(APR_HAS_PROCATTR_AUTOKILL_SET) // Our special preprocessor symbol isn't even defined -- wrong APR @@ -696,7 +700,7 @@ LLProcess::LLProcess(const LLSDOrParams& params): // take steps to terminate the child. This is all suspenders-and-belt: in // theory our destructor should kill an autokill child, but in practice // that doesn't always work (e.g. VWR-21538). - if (params.autokill) + if (mAutokill) { /*==========================================================================*| // NO: There may be an APR bug, not sure -- but at least on Mac, when @@ -799,7 +803,7 @@ LLProcess::~LLProcess() sProcessListener.dropPoll(*this); } - if (mAutokill) + if (mAttached) { kill("destructor"); } diff --git a/indra/llcommon/llprocess.h b/indra/llcommon/llprocess.h index bfac4567a5..e3386ad88e 100644 --- a/indra/llcommon/llprocess.h +++ b/indra/llcommon/llprocess.h @@ -167,6 +167,7 @@ public: args("args"), cwd("cwd"), autokill("autokill", true), + attached("attached", true), files("files"), postend("postend"), desc("desc") @@ -183,9 +184,31 @@ public: Multiple args; /// current working directory, if need it changed Optional cwd; - /// implicitly kill process on destruction of LLProcess object - /// (default true) + /// implicitly kill child process on termination of parent, whether + /// voluntary or crash (default true) Optional autokill; + /// implicitly kill process on destruction of LLProcess object + /// (default same as autokill) + /// + /// Originally, 'autokill' conflated two concepts: kill child process on + /// - destruction of its LLProcess object, and + /// - termination of parent process, voluntary or otherwise. + /// + /// It's useful to tease these apart. Some child processes are sent a + /// "clean up and terminate" message before the associated LLProcess + /// object is destroyed. A child process launched with attached=false + /// has an extra time window from the destruction of its LLProcess + /// until parent-process termination in which to perform its own + /// orderly shutdown, yet autokill=true still guarantees that we won't + /// accumulate orphan instances of such processes indefinitely. With + /// attached=true, if a child process cannot clean up between the + /// shutdown message and LLProcess destruction (presumably very soon + /// thereafter), it's forcibly killed anyway -- which can lead to + /// distressing user-visible crash indications. + /// + /// (The usefulness of attached=true with autokill=false is less + /// clear, but we don't prohibit that combination.) + Optional attached; /** * Up to three FileParam items: for child stdin, stdout, stderr. * Passing two FileParam entries means default treatment for stderr, @@ -540,7 +563,7 @@ private: std::string mDesc; std::string mPostend; apr_proc_t mProcess; - bool mAutokill; + bool mAutokill, mAttached; Status mStatus; // explicitly want this ptr_vector to be able to store NULLs typedef boost::ptr_vector< boost::nullable > PipeVector; diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp index 5ba343b183..b27e125d2e 100644 --- a/indra/llcommon/tests/llprocess_test.cpp +++ b/indra/llcommon/tests/llprocess_test.cpp @@ -788,6 +788,69 @@ namespace tut template<> template<> void object::test<10>() + { + set_test_name("attached=false"); + // almost just like autokill=false, except set autokill=true with + // attached=false. + NamedTempFile from("from", "not started"); + NamedTempFile to("to", ""); + LLProcess::handle phandle(0); + { + PythonProcessLauncher py(get_test_name(), + "from __future__ import with_statement\n" + "import sys, time\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ok')\n" + "# wait for 'go' from test program\n" + "for i in xrange(60):\n" + " time.sleep(1)\n" + " with open(sys.argv[2]) as f:\n" + " go = f.read()\n" + " if go == 'go':\n" + " break\n" + "else:\n" + " with open(sys.argv[1], 'w') as f:\n" + " f.write('never saw go')\n" + " sys.exit(1)\n" + "# okay, saw 'go', write 'ack'\n" + "with open(sys.argv[1], 'w') as f:\n" + " f.write('ack')\n"); + py.mParams.args.add(from.getName()); + py.mParams.args.add(to.getName()); + py.mParams.autokill = true; + py.mParams.attached = false; + py.launch(); + // Capture handle for later + phandle = py.mPy->getProcessHandle(); + // Wait for the script to wake up and do its first write + int i = 0, timeout = 60; + for ( ; i < timeout; ++i) + { + yield(); + if (readfile(from.getName(), "from autokill script") == "ok") + break; + } + // If we broke this loop because of the counter, something's wrong + ensure("script never started", i < timeout); + // Now destroy the LLProcess, which should NOT kill the child! + } + // If the destructor killed the child anyway, give it time to die + yield(2); + // How do we know it's not terminated? By making it respond to + // a specific stimulus in a specific way. + { + std::ofstream outf(to.getName().c_str()); + outf << "go"; + } // flush and close. + // now wait for the script to terminate... one way or another. + waitfor(phandle, "autokill script"); + // If the LLProcess destructor implicitly called kill(), the + // script could not have written 'ack' as we expect. + ensure_equals(get_test_name() + " script output", readfile(from.getName()), "ack"); + } + + template<> template<> + void object::test<11>() { set_test_name("'bogus' test"); CaptureLog recorder; @@ -801,7 +864,7 @@ namespace tut } template<> template<> - void object::test<11>() + void object::test<12>() { set_test_name("'file' test"); // Replace this test with one or more real 'file' tests when we @@ -815,7 +878,7 @@ namespace tut } template<> template<> - void object::test<12>() + void object::test<13>() { set_test_name("'tpipe' test"); // Replace this test with one or more real 'tpipe' tests when we @@ -832,7 +895,7 @@ namespace tut } template<> template<> - void object::test<13>() + void object::test<14>() { set_test_name("'npipe' test"); // Replace this test with one or more real 'npipe' tests when we @@ -850,7 +913,7 @@ namespace tut } template<> template<> - void object::test<14>() + void object::test<15>() { set_test_name("internal pipe name warning"); CaptureLog recorder; @@ -914,7 +977,7 @@ namespace tut } while (0) template<> template<> - void object::test<15>() + void object::test<16>() { set_test_name("get*Pipe() validation"); PythonProcessLauncher py(get_test_name(), @@ -934,7 +997,7 @@ namespace tut } template<> template<> - void object::test<16>() + void object::test<17>() { set_test_name("talk to stdin/stdout"); PythonProcessLauncher py(get_test_name(), @@ -992,7 +1055,7 @@ namespace tut } template<> template<> - void object::test<17>() + void object::test<18>() { set_test_name("listen for ReadPipe events"); PythonProcessLauncher py(get_test_name(), @@ -1052,7 +1115,7 @@ namespace tut } template<> template<> - void object::test<18>() + void object::test<19>() { set_test_name("ReadPipe \"eof\" event"); PythonProcessLauncher py(get_test_name(), @@ -1078,7 +1141,7 @@ namespace tut } template<> template<> - void object::test<19>() + void object::test<20>() { set_test_name("setLimit()"); PythonProcessLauncher py(get_test_name(), @@ -1107,7 +1170,7 @@ namespace tut } template<> template<> - void object::test<20>() + void object::test<21>() { set_test_name("peek() ReadPipe data"); PythonProcessLauncher py(get_test_name(), @@ -1160,7 +1223,7 @@ namespace tut } template<> template<> - void object::test<21>() + void object::test<22>() { set_test_name("bad postend"); std::string pumpname("postend"); @@ -1185,7 +1248,7 @@ namespace tut } template<> template<> - void object::test<22>() + void object::test<23>() { set_test_name("good postend"); PythonProcessLauncher py(get_test_name(), @@ -1241,7 +1304,7 @@ namespace tut }; template<> template<> - void object::test<23>() + void object::test<24>() { set_test_name("all data visible at postend"); PythonProcessLauncher py(get_test_name(), -- cgit v1.2.3 From d415e019a61cfd20bb1e254fbc9279f96047da85 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 24 Apr 2017 16:36:54 -0400 Subject: DRTVWR-418: Remove final shutdown cleanup as a cause of crashes. The recent LLSingleton work added a hook that would run during the C++ runtime's final destruction of static objects. When the LAST LLSingleton in any module was destroyed, its destructor would call LLSingletonBase::deleteAll(). That mechanism was intended to permit an application consuming LLSingletons to skip making an explicit deleteAll() call, knowing that all instantiated LLSingleton instances would eventually be cleaned up anyway. However -- experience proves that kicking off deleteAll() processing during the C++ runtime's final cleanup is too late. Too much has already been destroyed. That call tends to cause more shutdown crashes than it resolves. This commit deletes that whole mechanism. Going forward, if you want to clean up LLSingleton instances, you must explicitly call LLSingletonBase::deleteAll() during the application lifetime. If you don't, LLSingleton instances will simply be leaked -- which might be okay, considering the application is terminating anyway. --- indra/llcommon/llsingleton.cpp | 46 ++---------------------------------------- indra/llcommon/llsingleton.h | 30 ++++++++------------------- 2 files changed, 10 insertions(+), 66 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 9025e53bb2..18e1b96a5f 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -369,62 +369,20 @@ void LLSingletonBase::deleteAll() } } -/*------------------------ Final cleanup management ------------------------*/ -class LLSingletonBase::MasterRefcount -{ -public: - // store a POD int so it will be statically initialized to 0 - int refcount; -}; -static LLSingletonBase::MasterRefcount sMasterRefcount; - -LLSingletonBase::ref_ptr_t LLSingletonBase::get_master_refcount() -{ - // Calling this method constructs a new ref_ptr_t, which implicitly calls - // intrusive_ptr_add_ref(MasterRefcount*). - return &sMasterRefcount; -} - -void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount* mrc) -{ - // Count outstanding SingletonLifetimeManager instances. - ++mrc->refcount; -} - -void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) -{ - // Notice when each SingletonLifetimeManager instance is destroyed. - if (! --mrc->refcount) - { - // The last instance was destroyed. Time to kill any remaining - // LLSingletons -- but in dependency order. - LLSingletonBase::deleteAll(); - } -} - /*---------------------------- Logging helpers -----------------------------*/ namespace { bool oktolog() { // See comments in log() below. - return sMasterRefcount.refcount && LLError::is_available(); + return LLError::is_available(); } void log(LLError::ELevel level, const char* p1, const char* p2, const char* p3, const char* p4) { - // Check whether we're in the implicit final LLSingletonBase::deleteAll() - // call. We've carefully arranged for deleteAll() to be called when the - // last SingletonLifetimeManager instance is destroyed -- in other words, - // when the last translation unit containing an LLSingleton instance - // cleans up static data. That could happen after std::cerr is destroyed! // The is_available() test below ensures that we'll stop logging once // LLError has been cleaned up. If we had a similar portable test for - // std::cerr, this would be a good place to use it. As we do not, just - // don't log anything during implicit final deleteAll(). Detect that by - // the master refcount having gone to zero. - if (sMasterRefcount.refcount == 0) - return; + // std::cerr, this would be a good place to use it. // Check LLError::is_available() because some of LLError's infrastructure // is itself an LLSingleton. If that LLSingleton has not yet been diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 0d4a1f34f8..859e271e26 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -27,7 +27,6 @@ #include #include -#include #include #include #include @@ -36,8 +35,6 @@ class LLSingletonBase: private boost::noncopyable { public: class MasterList; - class MasterRefcount; - typedef boost::intrusive_ptr ref_ptr_t; private: // All existing LLSingleton instances are tracked in this master list. @@ -119,9 +116,6 @@ protected: const char* p3="", const char* p4=""); static std::string demangle(const char* mangled); - // obtain canonical ref_ptr_t - static ref_ptr_t get_master_refcount(); - // Default methods in case subclass doesn't declare them. virtual void initSingleton() {} virtual void cleanupSingleton() {} @@ -175,10 +169,6 @@ public: static void deleteAll(); }; -// support ref_ptr_t -void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount*); -void intrusive_ptr_release(LLSingletonBase::MasterRefcount*); - // Most of the time, we want LLSingleton_manage_master() to forward its // methods to real LLSingletonBase methods. template @@ -298,8 +288,7 @@ private: // stores pointer to singleton instance struct SingletonLifetimeManager { - SingletonLifetimeManager(): - mMasterRefcount(LLSingletonBase::get_master_refcount()) + SingletonLifetimeManager() { construct(); } @@ -317,17 +306,14 @@ private: // of static-object destruction, mean that we DO NOT WANT this // destructor to delete this LLSingleton. This destructor will run // without regard to any other LLSingleton whose cleanup might - // depend on its existence. What we really want is to count the - // runtime's attempts to cleanup LLSingleton static data -- and on - // the very last one, call LLSingletonBase::deleteAll(). That - // method will properly honor cross-LLSingleton dependencies. This - // is why we store an intrusive_ptr to a MasterRefcount: our - // ref_ptr_t member counts SingletonLifetimeManager instances. - // Once the runtime destroys the last of these, THEN we can delete - // every remaining LLSingleton. + // depend on its existence. If you want to clean up LLSingletons, + // call LLSingletonBase::deleteAll() sometime before static-object + // destruction begins. That method will properly honor cross- + // LLSingleton dependencies. Otherwise we simply leak LLSingleton + // instances at shutdown. Since the whole process is terminating + // anyway, that's not necessarily a bad thing; it depends on what + // resources your LLSingleton instances are managing. } - - LLSingletonBase::ref_ptr_t mMasterRefcount; }; protected: -- cgit v1.2.3 From 29dd5f0123a89f44688477d04a421b90d1efe635 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 27 Apr 2017 18:49:33 -0400 Subject: DRTVWR-418: Use (protected) LLSingleton to store "null instance" of LLSafeHandle's referenced type. Using LLSingleton gives us a well-defined time at which the "null instance" is deleted: LLSingletonBase::deleteAll(). --- indra/llcommon/llsafehandle.h | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsafehandle.h b/indra/llcommon/llsafehandle.h index af1c26dd4f..74d31297a0 100644 --- a/indra/llcommon/llsafehandle.h +++ b/indra/llcommon/llsafehandle.h @@ -27,6 +27,7 @@ #define LLSAFEHANDLE_H #include "llerror.h" // *TODO: consider eliminating this +#include "llsingleton.h" // Expands LLPointer to return a pointer to a special instance of class Type instead of NULL. // This is useful in instances where operations on NULL pointers are semantically safe and/or @@ -146,15 +147,24 @@ protected: } } - static Type* nonNull(Type* ptr) + // Define an LLSingleton whose sole purpose is to hold a "null instance" + // of the subject Type: the canonical instance to dereference if this + // LLSafeHandle actually holds a null pointer. We use LLSingleton + // specifically so that the "null instance" can be cleaned up at a well- + // defined time, specifically LLSingletonBase::deleteAll(). + // Of course, as with any LLSingleton, the "null instance" is only + // instantiated on demand -- in this case, if you actually try to + // dereference an LLSafeHandle containing null. + class NullInstanceHolder: public LLSingleton { - return ptr == NULL ? sNullFunc() : ptr; - } + LLSINGLETON_EMPTY_CTOR(NullInstanceHolder); + public: + Type mNullInstance; + }; - static Type* sNullFunc() + static Type* nonNull(Type* ptr) { - static Type sInstance; - return &sInstance; + return ptr? ptr : &NullInstanceHolder::instance().mNullInstance; } protected: -- cgit v1.2.3 From 52899ed62a241d7875277b0f3412e2be78f7b3af Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 2 May 2017 10:51:18 -0400 Subject: DRTVWR-418, MAINT-6996: Rationalize LLMemory wrt 64-bit support. There were two distinct LLMemory methods getCurrentRSS() and getWorkingSetSize(). It was pointless to have both: on Windows they were completely redundant; on other platforms getWorkingSetSize() always returned 0. (Amusingly, though the Windows implementations both made exactly the same GetProcessMemoryInfo() call and used exactly the same logic, the code was different in the two -- as though the second was implemented without awareness of the first, even though they were adjacent in the source file.) One of the actual MAINT-6996 problems was due to the fact that getWorkingSetSize() returned U32, where getCurrentRSS() returns U64. In other words, getWorkingSetSize() was both useless *and* wrong. Remove it, and change its one call to getCurrentRSS() instead. The other culprit was that in several places, the 64-bit WorkingSetSize returned by the Windows GetProcessMemoryInfo() call (and by getCurrentRSS()) was explicitly cast to a 32-bit data type. That works only when explicitly or implicitly (using LLUnits type conversion) scaling the value to kilobytes or megabytes. When the size in bytes is desired, use 64-bit types instead. In addition to the symptoms, LLMemory was overdue for a bit of cleanup. There was a 16K block of memory called reserveMem, the comment on which read: "reserve 16K for out of memory error handling." Yet *nothing* was ever done with that block! If it were going to be useful, one would think someone would at some point explicitly free the block. In fact there was a method freeReserve(), apparently for just that purpose -- which was never called. As things stood, reserveMem served only to *prevent* the viewer from ever using that chunk of memory. Remove reserveMem and the unused freeReserve(). The only function of initClass() and cleanupClass() was to allocate and free reserveMem. Remove initClass(), cleanupClass() and the LLCommon calls to them. In a similar vein, there was an LLMemoryInfo::getPhysicalMemoryClamped() method that returned U32Bytes. Its job was simply to return a size in bytes that could fit into a U32 data type, returning U32_MAX if the 64-bit value exceeded 4GB. Eliminate that; change all its calls to getPhysicalMemoryKB() (which getPhysicalMemoryClamped() used internally anyway). We no longer care about any platform that cannot handle 64-bit data types. --- indra/llcommon/llcommon.cpp | 2 - indra/llcommon/llmemory.cpp | 89 ++++++++------------------------------------- indra/llcommon/llmemory.h | 5 --- indra/llcommon/llsys.cpp | 16 -------- indra/llcommon/llsys.h | 5 --- 5 files changed, 15 insertions(+), 102 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcommon.cpp b/indra/llcommon/llcommon.cpp index 439ff4e628..2d665c611b 100644 --- a/indra/llcommon/llcommon.cpp +++ b/indra/llcommon/llcommon.cpp @@ -41,7 +41,6 @@ static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; //static void LLCommon::initClass() { - LLMemory::initClass(); if (!sAprInitialized) { ll_init_apr(); @@ -70,5 +69,4 @@ void LLCommon::cleanupClass() ll_cleanup_apr(); sAprInitialized = FALSE; } - SUBSYSTEM_CLEANUP(LLMemory); } diff --git a/indra/llcommon/llmemory.cpp b/indra/llcommon/llmemory.cpp index 1e04044269..6a7e1362f0 100644 --- a/indra/llcommon/llmemory.cpp +++ b/indra/llcommon/llmemory.cpp @@ -44,10 +44,10 @@ #include "llsys.h" #include "llframetimer.h" #include "lltrace.h" +#include "llerror.h" //---------------------------------------------------------------------------- //static -char* LLMemory::reserveMem = 0; U32Kilobytes LLMemory::sAvailPhysicalMemInKB(U32_MAX); U32Kilobytes LLMemory::sMaxPhysicalMemInKB(0); static LLTrace::SampleStatHandle sAllocatedMem("allocated_mem", "active memory in use by application"); @@ -78,29 +78,6 @@ void ll_assert_aligned_func(uintptr_t ptr,U32 alignment) #endif } -//static -void LLMemory::initClass() -{ - if (!reserveMem) - { - reserveMem = new char[16*1024]; // reserve 16K for out of memory error handling - } -} - -//static -void LLMemory::cleanupClass() -{ - delete [] reserveMem; - reserveMem = NULL; -} - -//static -void LLMemory::freeReserve() -{ - delete [] reserveMem; - reserveMem = NULL; -} - //static void LLMemory::initMaxHeapSizeGB(F32Gigabytes max_heap_size, BOOL prevent_heap_failure) { @@ -111,19 +88,18 @@ void LLMemory::initMaxHeapSizeGB(F32Gigabytes max_heap_size, BOOL prevent_heap_f //static void LLMemory::updateMemoryInfo() { -#if LL_WINDOWS - HANDLE self = GetCurrentProcess(); +#if LL_WINDOWS PROCESS_MEMORY_COUNTERS counters; - - if (!GetProcessMemoryInfo(self, &counters, sizeof(counters))) + + if (!GetProcessMemoryInfo(GetCurrentProcess(), &counters, sizeof(counters))) { LL_WARNS() << "GetProcessMemoryInfo failed" << LL_ENDL; return ; } - sAllocatedMemInKB = (U32Bytes)(counters.WorkingSetSize) ; + sAllocatedMemInKB = U64Bytes(counters.WorkingSetSize) ; sample(sAllocatedMem, sAllocatedMemInKB); - sAllocatedPageSizeInKB = (U32Bytes)(counters.PagefileUsage) ; + sAllocatedPageSizeInKB = U64Bytes(counters.PagefileUsage) ; sample(sVirtualMem, sAllocatedPageSizeInKB); U32Kilobytes avail_phys, avail_virtual; @@ -140,9 +116,9 @@ void LLMemory::updateMemoryInfo() } #else //not valid for other systems for now. - sAllocatedMemInKB = (U32Bytes)LLMemory::getCurrentRSS(); - sMaxPhysicalMemInKB = (U32Bytes)U32_MAX ; - sAvailPhysicalMemInKB = (U32Bytes)U32_MAX ; + sAllocatedMemInKB = U64Bytes(LLMemory::getCurrentRSS()); + sMaxPhysicalMemInKB = U64Bytes(U32_MAX); + sAvailPhysicalMemInKB = U64Bytes(U32_MAX); #endif return ; @@ -169,7 +145,7 @@ void* LLMemory::tryToAlloc(void* address, U32 size) return address ; #else return (void*)0x01 ; //skip checking -#endif +#endif } //static @@ -183,7 +159,7 @@ void LLMemory::logMemoryInfo(BOOL update) LL_INFOS() << "Current allocated physical memory(KB): " << sAllocatedMemInKB << LL_ENDL ; LL_INFOS() << "Current allocated page size (KB): " << sAllocatedPageSizeInKB << LL_ENDL ; - LL_INFOS() << "Current availabe physical memory(KB): " << sAvailPhysicalMemInKB << LL_ENDL ; + LL_INFOS() << "Current available physical memory(KB): " << sAvailPhysicalMemInKB << LL_ENDL ; LL_INFOS() << "Current max usable memory(KB): " << sMaxPhysicalMemInKB << LL_ENDL ; LL_INFOS() << "--- private pool information -- " << LL_ENDL ; @@ -263,12 +239,12 @@ U32Kilobytes LLMemory::getAllocatedMemKB() #if defined(LL_WINDOWS) +//static U64 LLMemory::getCurrentRSS() { - HANDLE self = GetCurrentProcess(); PROCESS_MEMORY_COUNTERS counters; - - if (!GetProcessMemoryInfo(self, &counters, sizeof(counters))) + + if (!GetProcessMemoryInfo(GetCurrentProcess(), &counters, sizeof(counters))) { LL_WARNS() << "GetProcessMemoryInfo failed" << LL_ENDL; return 0; @@ -277,20 +253,6 @@ U64 LLMemory::getCurrentRSS() return counters.WorkingSetSize; } -//static -U32 LLMemory::getWorkingSetSize() -{ - PROCESS_MEMORY_COUNTERS pmc ; - U32 ret = 0 ; - - if (GetProcessMemoryInfo( GetCurrentProcess(), &pmc, sizeof(pmc)) ) - { - ret = pmc.WorkingSetSize ; - } - - return ret ; -} - #elif defined(LL_DARWIN) /* @@ -337,11 +299,6 @@ U64 LLMemory::getCurrentRSS() return residentSize; } -U32 LLMemory::getWorkingSetSize() -{ - return 0 ; -} - #elif defined(LL_LINUX) U64 LLMemory::getCurrentRSS() @@ -353,7 +310,7 @@ U64 LLMemory::getCurrentRSS() if (fp == NULL) { LL_WARNS() << "couldn't open " << statPath << LL_ENDL; - goto bail; + return 0; } // Eee-yew! See Documentation/filesystems/proc.txt in your @@ -372,15 +329,9 @@ U64 LLMemory::getCurrentRSS() fclose(fp); -bail: return rss; } -U32 LLMemory::getWorkingSetSize() -{ - return 0 ; -} - #elif LL_SOLARIS #include #include @@ -410,11 +361,6 @@ U64 LLMemory::getCurrentRSS() return((U64)proc_psinfo.pr_rssize * 1024); } -U32 LLMemory::getWorkingSetSize() -{ - return 0 ; -} - #else U64 LLMemory::getCurrentRSS() @@ -422,11 +368,6 @@ U64 LLMemory::getCurrentRSS() return 0; } -U32 LLMemory::getWorkingSetSize() -{ - return 0; -} - #endif //-------------------------------------------------------------------------------------------------- diff --git a/indra/llcommon/llmemory.h b/indra/llcommon/llmemory.h index 5a3c9bd762..c37967e10e 100644 --- a/indra/llcommon/llmemory.h +++ b/indra/llcommon/llmemory.h @@ -334,13 +334,9 @@ inline void ll_memcpy_nonaliased_aligned_16(char* __restrict dst, const char* __ class LL_COMMON_API LLMemory { public: - static void initClass(); - static void cleanupClass(); - static void freeReserve(); // Return the resident set size of the current process, in bytes. // Return value is zero if not known. static U64 getCurrentRSS(); - static U32 getWorkingSetSize(); static void* tryToAlloc(void* address, U32 size); static void initMaxHeapSizeGB(F32Gigabytes max_heap_size, BOOL prevent_heap_failure); static void updateMemoryInfo() ; @@ -351,7 +347,6 @@ public: static U32Kilobytes getMaxMemKB() ; static U32Kilobytes getAllocatedMemKB() ; private: - static char* reserveMem; static U32Kilobytes sAvailPhysicalMemInKB ; static U32Kilobytes sMaxPhysicalMemInKB ; static U32Kilobytes sAllocatedMemInKB; diff --git a/indra/llcommon/llsys.cpp b/indra/llcommon/llsys.cpp index 1a66612e87..265c637b69 100644 --- a/indra/llcommon/llsys.cpp +++ b/indra/llcommon/llsys.cpp @@ -914,22 +914,6 @@ U32Kilobytes LLMemoryInfo::getPhysicalMemoryKB() const #endif } -U32Bytes LLMemoryInfo::getPhysicalMemoryClamped() const -{ - // Return the total physical memory in bytes, but clamp it - // to no more than U32_MAX - - U32Kilobytes phys_kb = getPhysicalMemoryKB(); - if (phys_kb >= U32Gigabytes(4)) - { - return U32Bytes(U32_MAX); - } - else - { - return phys_kb; - } -} - //static void LLMemoryInfo::getAvailableMemoryKB(U32Kilobytes& avail_physical_mem_kb, U32Kilobytes& avail_virtual_mem_kb) { diff --git a/indra/llcommon/llsys.h b/indra/llcommon/llsys.h index 962367f69f..95ef4cf065 100644 --- a/indra/llcommon/llsys.h +++ b/indra/llcommon/llsys.h @@ -113,11 +113,6 @@ public: void stream(std::ostream& s) const; ///< output text info to s U32Kilobytes getPhysicalMemoryKB() const; - - /*! Memory size in bytes, if total memory is >= 4GB then U32_MAX will - ** be returned. - */ - U32Bytes getPhysicalMemoryClamped() const; ///< Memory size in clamped bytes //get the available memory infomation in KiloBytes. static void getAvailableMemoryKB(U32Kilobytes& avail_physical_mem_kb, U32Kilobytes& avail_virtual_mem_kb); -- cgit v1.2.3 From 2bb19aec989d5964b22f64cc01aa7cbe962e1e6b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 2 May 2017 11:05:13 -0400 Subject: DRTVWR-418, MAINT-6996: Update Mac LLMemory::getCurrentRSS(). Evidently the Mac implementation of LLMemory::getCurrentRSS() goes back to OS X 10.3, because there was a helpful comment of the form: ------ The API used here is not capable of dealing with 64-bit memory sizes, but is available before 10.4. Once we start requiring 10.4, we can use the updated API, which looks like this: [new current implementation] Of course, this doesn't gain us anything unless we start building the viewer as a 64-bit executable, since that's the only way for our memory allocation to exceed 2^32. ------ Hey, guess what, we're building 64-bit viewers now! Thank you, whoever thoughtfully noted that, both for calling out the issue and sparing us the research. (The comment goes back to Subversion days, so hg blame shows only the merge-to-release changeset.) --- indra/llcommon/llmemory.cpp | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llmemory.cpp b/indra/llcommon/llmemory.cpp index 6a7e1362f0..9f9c3af892 100644 --- a/indra/llcommon/llmemory.cpp +++ b/indra/llcommon/llmemory.cpp @@ -255,19 +255,6 @@ U64 LLMemory::getCurrentRSS() #elif defined(LL_DARWIN) -/* - The API used here is not capable of dealing with 64-bit memory sizes, but is available before 10.4. - - Once we start requiring 10.4, we can use the updated API, which looks like this: - - task_basic_info_64_data_t basicInfo; - mach_msg_type_number_t basicInfoCount = TASK_BASIC_INFO_64_COUNT; - if (task_info(mach_task_self(), TASK_BASIC_INFO_64, (task_info_t)&basicInfo, &basicInfoCount) == KERN_SUCCESS) - - Of course, this doesn't gain us anything unless we start building the viewer as a 64-bit executable, since that's the only way - for our memory allocation to exceed 2^32. -*/ - // if (sysctl(ctl, 2, &page_size, &size, NULL, 0) == -1) // { // LL_WARNS() << "Couldn't get page size" << LL_ENDL; @@ -280,9 +267,9 @@ U64 LLMemory::getCurrentRSS() U64 LLMemory::getCurrentRSS() { U64 residentSize = 0; - task_basic_info_data_t basicInfo; - mach_msg_type_number_t basicInfoCount = TASK_BASIC_INFO_COUNT; - if (task_info(mach_task_self(), TASK_BASIC_INFO, (task_info_t)&basicInfo, &basicInfoCount) == KERN_SUCCESS) + task_basic_info_64_data_t basicInfo; + mach_msg_type_number_t basicInfoCount = TASK_BASIC_INFO_64_COUNT; + if (task_info(mach_task_self(), TASK_BASIC_INFO_64, (task_info_t)&basicInfo, &basicInfoCount) == KERN_SUCCESS) { residentSize = basicInfo.resident_size; -- cgit v1.2.3 From 9002e3cbc7a4f7ef2847ab6652de0f11eaff6ef2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 3 May 2017 13:20:56 -0400 Subject: DRTVWR-418: Add big deprecation notice to llsafehandle.h. --- indra/llcommon/llsafehandle.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsafehandle.h b/indra/llcommon/llsafehandle.h index 74d31297a0..968a5cfeb1 100644 --- a/indra/llcommon/llsafehandle.h +++ b/indra/llcommon/llsafehandle.h @@ -29,6 +29,29 @@ #include "llerror.h" // *TODO: consider eliminating this #include "llsingleton.h" +/*==========================================================================*| + ____ ___ _ _ ___ _____ _ _ ____ _____ _ +| _ \ / _ \ | \ | |/ _ \_ _| | | | / ___|| ____| | +| | | | | | | | \| | | | || | | | | \___ \| _| | | +| |_| | |_| | | |\ | |_| || | | |_| |___) | |___|_| +|____/ \___/ |_| \_|\___/ |_| \___/|____/|_____(_) + +This handle class is deprecated. Unfortunately it is already in widespread use +to reference the LLObjectSelection and LLParcelSelection classes, but do not +apply LLSafeHandle to other classes, or declare new instances. + +Instead, use LLPointer or other smart pointer types with appropriate checks +for NULL. If you're certain the reference cannot (or must not) be NULL, +consider storing a C++ reference instead -- or use (e.g.) LLCheckedHandle. + +When an LLSafeHandle containing NULL is dereferenced, it resolves to a +canonical "null" T instance. This raises issues about the lifespan of the +"null" instance. In addition to encouraging sloppy coding practices, it +potentially masks bugs when code that performs some mutating operation +inadvertently applies it to the "null" instance. That result might or might +not ever affect subsequent computations. +|*==========================================================================*/ + // Expands LLPointer to return a pointer to a special instance of class Type instead of NULL. // This is useful in instances where operations on NULL pointers are semantically safe and/or // when error checking occurs at a different granularity or in a different part of the code -- cgit v1.2.3 From d6b870c0cae04a9a2576c8849ebe03822eb78e6c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 3 May 2017 22:53:34 -0400 Subject: DRTVWR-418: Add dtor to LLSafeHandle::NullInstanceHolder to suppress fatal warnings in Visual Studio. --- indra/llcommon/llsafehandle.h | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsafehandle.h b/indra/llcommon/llsafehandle.h index 968a5cfeb1..9550e6253e 100644 --- a/indra/llcommon/llsafehandle.h +++ b/indra/llcommon/llsafehandle.h @@ -181,6 +181,7 @@ protected: class NullInstanceHolder: public LLSingleton { LLSINGLETON_EMPTY_CTOR(NullInstanceHolder); + ~NullInstanceHolder() {} public: Type mNullInstance; }; -- cgit v1.2.3