From 464a0df4c1d3e4073fe0bde506ac1d4aa194b02f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 Jun 2016 16:51:50 -0400 Subject: DRTVWR-418: Unify control flow through LLAppViewer across platforms. The LLApp API used to consist of init(), mainLoop(), cleanup() methods. This makes sense -- but on Mac that structure was being subverted. The method called mainLoop() was in fact being called once per frame. There was initialization code in the method, which (on Mac) needed to be skipped with an already-initialized bool. There was a 'while' loop which (on Mac) needed to be turned into an 'if' instead so the method would return after every frame. Rename LLApp::mainLoop() to frame(). Propagate through subclasses LLAppViewer and LLCrashLogger. Document the fact that frame() returns true to mean "done." (This was always the case, but had to be inferred from the code.) Rename the Mac Objective-C function mainLoop to oneFrame. Rename the C++ free function it calls from runMainLoop() to pumpMainLoop(). Add comments to llappdelegate-objc.mm explaining (inferred) control flow. Change the Linux viewer main() and the Windows viewer WINMAIN() from a single LLAppViewer::mainLoop() call to repeatedly call frame() until it returns true. Move initialization code from the top of LLAppViewer::frame() to the init() method, where it more properly belongs. Remove corresponding mMainLoopInitialized flag (and all references) from LLAppViewer. Remove 'while (! LLApp::isExiting())' (or on Mac, 'if (! LLApp::isExiting())') from LLAppViewer::frame() -- thus unindenting the whole body of the 'while' and causing many lines of apparent change. (Apologies to reviewers.) There are four LLApp states: APP_STATUS_RUNNING, APP_STATUS_QUITTING, APP_STATUS_STOPPED and APP_STATUS_ERROR. Change LLAppViewer::frame() return value from (isExiting()) (QUITTING or ERROR) to (! isRunning()). I do not know under what circumstances the state might transition to STOPPED during a frame() call, but I'm quite sure that if it does, we don't want to call frame() again. We only want a subsequent call if the state is RUNNING. Also rename mainLoop() method in LLCrashLogger subclasses LLCrashLoggerWindows, LLCrashLoggerMac, LLCrashLoggerLinux. Of course it's completely up to the frame() method whether to yield control; none of those in fact do. Honor protocol by returning true (frame() is done), even though each one's main() caller ignores the return value. In fact LLCrashLoggerWindows::mainLoop() wasn't using the return protocol correctly anyway, returning wParam or 0 or 1 -- possibly because the return protocol was never explicitly documented. It should always return true: "I'm done, don't call me again." --- indra/newview/llappdelegate-objc.mm | 24 +- indra/newview/llappviewer.cpp | 456 +++++++++++++++++------------------- indra/newview/llappviewer.h | 3 +- indra/newview/llappviewerlinux.cpp | 6 +- indra/newview/llappviewermacosx.cpp | 9 +- indra/newview/llappviewerwin32.cpp | 6 +- 6 files changed, 243 insertions(+), 261 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 549df80fa1..be8877328d 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -48,16 +48,19 @@ - (void) applicationDidFinishLaunching:(NSNotification *)notification { frameTimer = nil; - + [self languageUpdated]; - + if (initViewer()) { - frameTimer = [NSTimer scheduledTimerWithTimeInterval:0.0 target:self selector:@selector(mainLoop) userInfo:nil repeats:YES]; + // Set up recurring calls to oneFrame (repeating timer with timeout 0) + // until applicationShouldTerminate. + frameTimer = [NSTimer scheduledTimerWithTimeInterval:0.0 target:self + selector:@selector(oneFrame) userInfo:nil repeats:YES]; } else { handleQuit(); } - + [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(languageUpdated) name:@"NSTextInputContextKeyboardSelectionDidChangeNotification" object:nil]; // [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self andSelector:@selector(handleGetURLEvent:withReplyEvent:) forEventClass:kInternetEventClass andEventID:kAEGetURL]; @@ -96,22 +99,29 @@ - (NSApplicationDelegateReply) applicationShouldTerminate:(NSApplication *)sender { - if (!runMainLoop()) + // run one frame to assess state + if (!pumpMainLoop()) { + // pumpMainLoop() returns true when done, false if it wants to be + // called again. Since it returned false, do not yet cancel + // frameTimer. handleQuit(); return NSTerminateCancel; } else { + // pumpMainLoop() returned true: it's done. Okay, done with frameTimer. [frameTimer release]; cleanupViewer(); return NSTerminateNow; } } -- (void) mainLoop +- (void) oneFrame { - bool appExiting = runMainLoop(); + bool appExiting = pumpMainLoop(); if (appExiting) { + // Once pumpMainLoop() reports that we're done, cancel frameTimer: + // stop the repetitive calls. [frameTimer release]; [[NSApplication sharedApplication] terminate:self]; } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index b6d02ea2f8..604e45f314 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -769,9 +769,6 @@ bool LLAppViewer::init() // // Start of the application // -#ifdef LL_DARWIN - mMainLoopInitialized = false; -#endif // initialize LLWearableType translation bridge. // Memory will be cleaned up in ::cleanupClass() @@ -1220,6 +1217,23 @@ bool LLAppViewer::init() boost::bind(&LLControlGroup::getU32, boost::ref(gSavedSettings), _1), boost::bind(&LLControlGroup::declareU32, boost::ref(gSavedSettings), _1, _2, _3, LLControlVariable::PERSIST_ALWAYS)); + /*----------------------------------------------------------------------*/ + // nat 2016-06-29 moved the following here from the former mainLoop(). + mMainloopTimeout = new LLWatchdogTimeout(); + + // Create IO Pump to use for HTTP Requests. + gServicePump = new LLPumpIO(gAPRPoolp); + + // Note: this is where gLocalSpeakerMgr and gActiveSpeakerMgr used to be instantiated. + + LLVoiceChannel::initClass(); + LLVoiceClient::getInstance()->init(gServicePump); + LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&LLFloaterIMContainer::onCurrentChannelChanged, _1), true); + + joystick = LLViewerJoystick::getInstance(); + joystick->setNeedsReset(true); + /*----------------------------------------------------------------------*/ + return true; } @@ -1294,297 +1308,255 @@ static LLTrace::BlockTimerStatHandle FTM_AGENT_UPDATE("Update"); // externally visible timers LLTrace::BlockTimerStatHandle FTM_FRAME("Frame"); -bool LLAppViewer::mainLoop() +bool LLAppViewer::frame() { -#ifdef LL_DARWIN - if (!mMainLoopInitialized) -#endif - { - LL_INFOS() << "Entering main_loop" << LL_ENDL; - mMainloopTimeout = new LLWatchdogTimeout(); - - //------------------------------------------- - // Run main loop until time to quit - //------------------------------------------- - - // Create IO Pump to use for HTTP Requests. - gServicePump = new LLPumpIO(gAPRPoolp); - - // Note: this is where gLocalSpeakerMgr and gActiveSpeakerMgr used to be instantiated. - - LLVoiceChannel::initClass(); - LLVoiceClient::getInstance()->init(gServicePump); - LLVoiceChannel::setCurrentVoiceChannelChangedCallback(boost::bind(&LLFloaterIMContainer::onCurrentChannelChanged, _1), true); - - joystick = LLViewerJoystick::getInstance(); - joystick->setNeedsReset(true); - -#ifdef LL_DARWIN - // Ensure that this section of code never gets called again on OS X. - mMainLoopInitialized = true; -#endif - } - // As we do not (yet) send data on the mainloop LLEventPump that varies - // with each frame, no need to instantiate a new LLSD event object each - // time. Obviously, if that changes, just instantiate the LLSD at the - // point of posting. - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - - LLSD newFrame; - + LLSD newFrame; + LLTimer frameTimer,idleTimer; LLTimer debugTime; - + //LLPrivateMemoryPoolTester::getInstance()->run(false) ; //LLPrivateMemoryPoolTester::getInstance()->run(true) ; //LLPrivateMemoryPoolTester::destroy() ; - // Handle messages -#ifdef LL_DARWIN - if (!LLApp::isExiting()) -#else - while (!LLApp::isExiting()) -#endif + LL_RECORD_BLOCK_TIME(FTM_FRAME); + LLTrace::BlockTimer::processTimes(); + LLTrace::get_frame_recording().nextPeriod(); + LLTrace::BlockTimer::logStats(); + + LLTrace::get_thread_recorder()->pullFromChildren(); + + //clear call stack records + LL_CLEAR_CALLSTACKS(); + + //check memory availability information + checkMemory() ; + + try { - LL_RECORD_BLOCK_TIME(FTM_FRAME); - LLTrace::BlockTimer::processTimes(); - LLTrace::get_frame_recording().nextPeriod(); - LLTrace::BlockTimer::logStats(); + pingMainloopTimeout("Main:MiscNativeWindowEvents"); + + if (gViewerWindow) + { + LL_RECORD_BLOCK_TIME(FTM_MESSAGES); + gViewerWindow->getWindow()->processMiscNativeEvents(); + } - LLTrace::get_thread_recorder()->pullFromChildren(); + pingMainloopTimeout("Main:GatherInput"); - //clear call stack records - LL_CLEAR_CALLSTACKS(); + if (gViewerWindow) + { + LL_RECORD_BLOCK_TIME(FTM_MESSAGES); + if (!restoreErrorTrap()) + { + LL_WARNS() << " Someone took over my signal/exception handler (post messagehandling)!" << LL_ENDL; + } - //check memory availability information - checkMemory() ; + gViewerWindow->getWindow()->gatherInput(); + } + +#if 1 && !LL_RELEASE_FOR_DOWNLOAD + // once per second debug info + if (debugTime.getElapsedTimeF32() > 1.f) + { + debugTime.reset(); + } - try +#endif + //memory leaking simulation + LLFloaterMemLeak* mem_leak_instance = + LLFloaterReg::findTypedInstance("mem_leaking"); + if(mem_leak_instance) { - pingMainloopTimeout("Main:MiscNativeWindowEvents"); + mem_leak_instance->idle() ; + } + + // canonical per-frame event + mainloop.post(newFrame); - if (gViewerWindow) + if (!LLApp::isExiting()) + { + pingMainloopTimeout("Main:JoystickKeyboard"); + + // Scan keyboard for movement keys. Command keys and typing + // are handled by windows callbacks. Don't do this until we're + // done initializing. JC + if ((gHeadlessClient || gViewerWindow->getWindow()->getVisible()) + && gViewerWindow->getActive() + && !gViewerWindow->getWindow()->getMinimized() + && LLStartUp::getStartupState() == STATE_STARTED + && (gHeadlessClient || !gViewerWindow->getShowProgress()) + && !gFocusMgr.focusLocked()) { - LL_RECORD_BLOCK_TIME(FTM_MESSAGES); - gViewerWindow->getWindow()->processMiscNativeEvents(); + joystick->scanJoystick(); + gKeyboard->scanKeyboard(); } - - pingMainloopTimeout("Main:GatherInput"); - - if (gViewerWindow) + + // Update state based on messages, user input, object idle. { - LL_RECORD_BLOCK_TIME(FTM_MESSAGES); - if (!restoreErrorTrap()) - { - LL_WARNS() << " Someone took over my signal/exception handler (post messagehandling)!" << LL_ENDL; - } + pauseMainloopTimeout(); // *TODO: Remove. Messages shouldn't be stalling for 20+ seconds! + + LL_RECORD_BLOCK_TIME(FTM_IDLE); + idle(); - gViewerWindow->getWindow()->gatherInput(); + resumeMainloopTimeout(); } -#if 1 && !LL_RELEASE_FOR_DOWNLOAD - // once per second debug info - if (debugTime.getElapsedTimeF32() > 1.f) + if (gDoDisconnect && (LLStartUp::getStartupState() == STATE_STARTED)) { - debugTime.reset(); + pauseMainloopTimeout(); + saveFinalSnapshot(); + disconnectViewer(); + resumeMainloopTimeout(); } - -#endif - //memory leaking simulation - LLFloaterMemLeak* mem_leak_instance = - LLFloaterReg::findTypedInstance("mem_leaking"); - if(mem_leak_instance) - { - mem_leak_instance->idle() ; - } - - // canonical per-frame event - mainloop.post(newFrame); - if (!LLApp::isExiting()) + // Render scene. + // *TODO: Should we run display() even during gHeadlessClient? DK 2011-02-18 + if (!LLApp::isExiting() && !gHeadlessClient) { - pingMainloopTimeout("Main:JoystickKeyboard"); - - // Scan keyboard for movement keys. Command keys and typing - // are handled by windows callbacks. Don't do this until we're - // done initializing. JC - if ((gHeadlessClient || gViewerWindow->getWindow()->getVisible()) - && gViewerWindow->getActive() - && !gViewerWindow->getWindow()->getMinimized() - && LLStartUp::getStartupState() == STATE_STARTED - && (gHeadlessClient || !gViewerWindow->getShowProgress()) - && !gFocusMgr.focusLocked()) - { - joystick->scanJoystick(); - gKeyboard->scanKeyboard(); - } + pingMainloopTimeout("Main:Display"); + gGLActive = TRUE; + display(); + pingMainloopTimeout("Main:Snapshot"); + LLFloaterSnapshot::update(); // take snapshots + gGLActive = FALSE; + } + } - // Update state based on messages, user input, object idle. - { - pauseMainloopTimeout(); // *TODO: Remove. Messages shouldn't be stalling for 20+ seconds! - - LL_RECORD_BLOCK_TIME(FTM_IDLE); - idle(); + pingMainloopTimeout("Main:Sleep"); - resumeMainloopTimeout(); - } - - if (gDoDisconnect && (LLStartUp::getStartupState() == STATE_STARTED)) - { - pauseMainloopTimeout(); - saveFinalSnapshot(); - disconnectViewer(); - resumeMainloopTimeout(); - } + pauseMainloopTimeout(); - // Render scene. - // *TODO: Should we run display() even during gHeadlessClient? DK 2011-02-18 - if (!LLApp::isExiting() && !gHeadlessClient) + // Sleep and run background threads + { + LL_RECORD_BLOCK_TIME(FTM_SLEEP); + + // yield some time to the os based on command line option + if(mYieldTime >= 0) + { + LL_RECORD_BLOCK_TIME(FTM_YIELD); + ms_sleep(mYieldTime); + } + + // yield cooperatively when not running as foreground window + if ( (gViewerWindow && !gViewerWindow->getWindow()->getVisible()) + || !gFocusMgr.getAppHasFocus()) + { + // Sleep if we're not rendering, or the window is minimized. + S32 milliseconds_to_sleep = llclamp(gSavedSettings.getS32("BackgroundYieldTime"), 0, 1000); + // don't sleep when BackgroundYieldTime set to 0, since this will still yield to other threads + // of equal priority on Windows + if (milliseconds_to_sleep > 0) { - pingMainloopTimeout("Main:Display"); - gGLActive = TRUE; - display(); - pingMainloopTimeout("Main:Snapshot"); - LLFloaterSnapshot::update(); // take snapshots - gGLActive = FALSE; + ms_sleep(milliseconds_to_sleep); + // also pause worker threads during this wait period + LLAppViewer::getTextureCache()->pause(); + LLAppViewer::getImageDecodeThread()->pause(); } } - - pingMainloopTimeout("Main:Sleep"); - pauseMainloopTimeout(); + if (mRandomizeFramerate) + { + ms_sleep(rand() % 200); + } - // Sleep and run background threads + if (mPeriodicSlowFrame + && (gFrameCount % 10 == 0)) { - LL_RECORD_BLOCK_TIME(FTM_SLEEP); - - // yield some time to the os based on command line option - if(mYieldTime >= 0) - { - LL_RECORD_BLOCK_TIME(FTM_YIELD); - ms_sleep(mYieldTime); - } + LL_INFOS() << "Periodic slow frame - sleeping 500 ms" << LL_ENDL; + ms_sleep(500); + } + + const F64Milliseconds max_idle_time = llmin(.005f*10.f*(F32Milliseconds)gFrameTimeSeconds, F32Milliseconds(5)); // 5 ms a second + idleTimer.reset(); + S32 total_work_pending = 0; + S32 total_io_pending = 0; + while(1) + { + S32 work_pending = 0; + S32 io_pending = 0; + F32 max_time = llmin(gFrameIntervalSeconds.value() *10.f, 1.f); + + work_pending += updateTextureThreads(max_time); - // yield cooperatively when not running as foreground window - if ( (gViewerWindow && !gViewerWindow->getWindow()->getVisible()) - || !gFocusMgr.getAppHasFocus()) { - // Sleep if we're not rendering, or the window is minimized. - S32 milliseconds_to_sleep = llclamp(gSavedSettings.getS32("BackgroundYieldTime"), 0, 1000); - // don't sleep when BackgroundYieldTime set to 0, since this will still yield to other threads - // of equal priority on Windows - if (milliseconds_to_sleep > 0) - { - ms_sleep(milliseconds_to_sleep); - // also pause worker threads during this wait period - LLAppViewer::getTextureCache()->pause(); - LLAppViewer::getImageDecodeThread()->pause(); - } + LL_RECORD_BLOCK_TIME(FTM_VFS); + io_pending += LLVFSThread::updateClass(1); } - - if (mRandomizeFramerate) { - ms_sleep(rand() % 200); + LL_RECORD_BLOCK_TIME(FTM_LFS); + io_pending += LLLFSThread::updateClass(1); } - if (mPeriodicSlowFrame - && (gFrameCount % 10 == 0)) + if (io_pending > 1000) { - LL_INFOS() << "Periodic slow frame - sleeping 500 ms" << LL_ENDL; - ms_sleep(500); + ms_sleep(llmin(io_pending/100,100)); // give the vfs some time to catch up } - const F64Milliseconds max_idle_time = llmin(.005f*10.f*(F32Milliseconds)gFrameTimeSeconds, F32Milliseconds(5)); // 5 ms a second - idleTimer.reset(); - S32 total_work_pending = 0; - S32 total_io_pending = 0; - while(1) - { - S32 work_pending = 0; - S32 io_pending = 0; - F32 max_time = llmin(gFrameIntervalSeconds.value() *10.f, 1.f); - - work_pending += updateTextureThreads(max_time); - - { - LL_RECORD_BLOCK_TIME(FTM_VFS); - io_pending += LLVFSThread::updateClass(1); - } - { - LL_RECORD_BLOCK_TIME(FTM_LFS); - io_pending += LLLFSThread::updateClass(1); - } - - if (io_pending > 1000) - { - ms_sleep(llmin(io_pending/100,100)); // give the vfs some time to catch up - } - - total_work_pending += work_pending ; - total_io_pending += io_pending ; - - if (!work_pending || idleTimer.getElapsedTimeF64() >= max_idle_time) - { - break; - } - } - gMeshRepo.update() ; + total_work_pending += work_pending ; + total_io_pending += io_pending ; - if(!total_work_pending) //pause texture fetching threads if nothing to process. + if (!work_pending || idleTimer.getElapsedTimeF64() >= max_idle_time) { - LLAppViewer::getTextureCache()->pause(); - LLAppViewer::getImageDecodeThread()->pause(); - LLAppViewer::getTextureFetch()->pause(); - } - if(!total_io_pending) //pause file threads if nothing to process. - { - LLVFSThread::sLocal->pause(); - LLLFSThread::sLocal->pause(); - } - - //texture fetching debugger - if(LLTextureFetchDebugger::isEnabled()) - { - LLFloaterTextureFetchDebugger* tex_fetch_debugger_instance = - LLFloaterReg::findTypedInstance("tex_fetch_debugger"); - if(tex_fetch_debugger_instance) - { - tex_fetch_debugger_instance->idle() ; - } + break; } + } + gMeshRepo.update() ; + + if(!total_work_pending) //pause texture fetching threads if nothing to process. + { + LLAppViewer::getTextureCache()->pause(); + LLAppViewer::getImageDecodeThread()->pause(); + LLAppViewer::getTextureFetch()->pause(); + } + if(!total_io_pending) //pause file threads if nothing to process. + { + LLVFSThread::sLocal->pause(); + LLLFSThread::sLocal->pause(); + } - if ((LLStartUp::getStartupState() >= STATE_CLEANUP) && - (frameTimer.getElapsedTimeF64() > FRAME_STALL_THRESHOLD)) + //texture fetching debugger + if(LLTextureFetchDebugger::isEnabled()) + { + LLFloaterTextureFetchDebugger* tex_fetch_debugger_instance = + LLFloaterReg::findTypedInstance("tex_fetch_debugger"); + if(tex_fetch_debugger_instance) { - gFrameStalls++; + tex_fetch_debugger_instance->idle() ; } - frameTimer.reset(); - - resumeMainloopTimeout(); - - pingMainloopTimeout("Main:End"); - } - } - catch(std::bad_alloc) - { - LLMemory::logMemoryInfo(TRUE) ; + } - //stop memory leaking simulation - LLFloaterMemLeak* mem_leak_instance = - LLFloaterReg::findTypedInstance("mem_leaking"); - if(mem_leak_instance) + if ((LLStartUp::getStartupState() >= STATE_CLEANUP) && + (frameTimer.getElapsedTimeF64() > FRAME_STALL_THRESHOLD)) { - mem_leak_instance->stop() ; - LL_WARNS() << "Bad memory allocation in LLAppViewer::mainLoop()!" << LL_ENDL ; + gFrameStalls++; } - else - { - //output possible call stacks to log file. - LLError::LLCallStacks::print() ; + frameTimer.reset(); - LL_ERRS() << "Bad memory allocation in LLAppViewer::mainLoop()!" << LL_ENDL ; - } + resumeMainloopTimeout(); + + pingMainloopTimeout("Main:End"); + } + } + catch(std::bad_alloc) + { + LLMemory::logMemoryInfo(TRUE) ; + + //stop memory leaking simulation + LLFloaterMemLeak* mem_leak_instance = + LLFloaterReg::findTypedInstance("mem_leaking"); + if(mem_leak_instance) + { + mem_leak_instance->stop() ; + LL_WARNS() << "Bad memory allocation in LLAppViewer::frame()!" << LL_ENDL ; + } + else + { + //output possible call stacks to log file. + LLError::LLCallStacks::print() ; + + LL_ERRS() << "Bad memory allocation in LLAppViewer::frame()!" << LL_ENDL ; } } @@ -1618,7 +1590,7 @@ bool LLAppViewer::mainLoop() LL_INFOS() << "Exiting main_loop" << LL_ENDL; } - return LLApp::isExiting(); + return ! LLApp::isRunning(); } S32 LLAppViewer::updateTextureThreads(F32 max_time) diff --git a/indra/newview/llappviewer.h b/indra/newview/llappviewer.h index b5e674bd7b..f7c1bb58b4 100644 --- a/indra/newview/llappviewer.h +++ b/indra/newview/llappviewer.h @@ -79,7 +79,7 @@ public: // virtual bool init(); // Override to do application initialization virtual bool cleanup(); // Override to do application cleanup - virtual bool mainLoop(); // Override for the application main loop. Needs to at least gracefully notice the QUITTING state and exit. + virtual bool frame(); // Override for application body logic // Application control void flushVFSIO(); // waits for vfs transfers to complete @@ -283,7 +283,6 @@ private: std::string mSerialNumber; bool mPurgeCache; bool mPurgeOnExit; - bool mMainLoopInitialized; LLViewerJoystick* joystick; bool mSavedFinalSnapshot; diff --git a/indra/newview/llappviewerlinux.cpp b/indra/newview/llappviewerlinux.cpp index f5742b29cf..6f32aab851 100644 --- a/indra/newview/llappviewerlinux.cpp +++ b/indra/newview/llappviewerlinux.cpp @@ -95,10 +95,8 @@ int main( int argc, char **argv ) } // Run the application main loop - if(!LLApp::isQuitting()) - { - viewer_app_ptr->mainLoop(); - } + while (! viewer_app_ptr->frame()) + {} if (!LLApp::isError()) { diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index ca219fda59..4fe1e31668 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -117,12 +117,17 @@ void handleQuit() LLAppViewer::instance()->userQuit(); } -bool runMainLoop() +// This function is called pumpMainLoop() rather than runMainLoop() because +// it passes control to the viewer's main-loop logic for a single frame. Like +// LLAppViewer::frame(), it returns 'true' when it's done. Until then, it +// expects to be called again by the timer in LLAppDelegate +// (llappdelegate-objc.mm). +bool pumpMainLoop() { bool ret = LLApp::isQuitting(); if (!ret && gViewerAppPtr != NULL) { - ret = gViewerAppPtr->mainLoop(); + ret = gViewerAppPtr->frame(); } else { ret = true; } diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index 4786f83bfd..a7f248ab5a 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -317,10 +317,8 @@ int APIENTRY WINMAIN(HINSTANCE hInstance, } // Run the application main loop - if(!LLApp::isQuitting()) - { - viewer_app_ptr->mainLoop(); - } + while (! viewer_app_ptr->frame()) + {} if (!LLApp::isError()) { -- cgit v1.2.3 From b031b1a6255af0813698bd40586f30f7b0a76f58 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Jul 2016 10:43:36 -0400 Subject: MAINT-5011: Derive remaining exception classes from std::exception. In particular: NotImplemented in llhttpnode.cpp RelocateError in llupdateinstaller.cpp LLProtectedDataException, LLCertException and subclasses in llsecapi.h Had to add no-throw destructor overrides to LLCertException and subclasses because otherwise clang complains that the implicitly-generated destructor's exception specification is more lax than the base class's. --- indra/newview/llsecapi.cpp | 2 +- indra/newview/llsecapi.h | 34 +++++++++++++++++++--------------- 2 files changed, 20 insertions(+), 16 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index 4f9f83b6f2..c27709a57b 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -69,7 +69,7 @@ void initializeSecHandler() } if (!exception_msg.empty()) // an exception was thrown. { - throw LLProtectedDataException(exception_msg.c_str()); + throw LLProtectedDataException(exception_msg); } } diff --git a/indra/newview/llsecapi.h b/indra/newview/llsecapi.h index 6fe3ee31cf..02438f77b7 100644 --- a/indra/newview/llsecapi.h +++ b/indra/newview/llsecapi.h @@ -32,6 +32,7 @@ #include #include #include "llpointer.h" +#include #ifdef LL_WINDOWS #pragma warning(disable:4250) @@ -116,17 +117,14 @@ -class LLProtectedDataException +struct LLProtectedDataException: public std::runtime_error { -public: - LLProtectedDataException(const char *msg) + LLProtectedDataException(const std::string& msg): + std::runtime_error(msg) { - LL_WARNS("SECAPI") << "Protected Data Error: " << (std::string)msg << LL_ENDL; - mMsg = (std::string)msg; + LL_WARNS("SECAPI") << "Protected Data Error: " << msg << LL_ENDL; } - std::string getMessage() { return mMsg; } -protected: - std::string mMsg; + std::string getMessage() { return what(); } }; // class LLCertificate @@ -334,22 +332,22 @@ std::ostream& operator <<(std::ostream& s, const LLCredential& cred); // All error handling is via exceptions. -class LLCertException +class LLCertException: public std::runtime_error { public: - LLCertException(LLPointer cert, const char* msg) + LLCertException(LLPointer cert, const std::string& msg): + std::runtime_error(msg) { mCert = cert; - LL_WARNS("SECAPI") << "Certificate Error: " << (std::string)msg << LL_ENDL; - mMsg = (std::string)msg; + LL_WARNS("SECAPI") << "Certificate Error: " << msg << LL_ENDL; } + virtual ~LLCertException() throw() {} LLPointer getCert() { return mCert; } - std::string getMessage() { return mMsg; } + std::string getMessage() { return what(); } protected: LLPointer mCert; - std::string mMsg; }; class LLInvalidCertificate : public LLCertException @@ -358,6 +356,7 @@ public: LLInvalidCertificate(LLPointer cert) : LLCertException(cert, "CertInvalid") { } + virtual ~LLInvalidCertificate() throw() {} protected: }; @@ -367,6 +366,7 @@ public: LLCertValidationTrustException(LLPointer cert) : LLCertException(cert, "CertUntrusted") { } + virtual ~LLCertValidationTrustException() throw() {} protected: }; @@ -378,7 +378,7 @@ public: { mHostname = hostname; } - + virtual ~LLCertValidationHostnameException() throw() {} std::string getHostname() { return mHostname; } protected: std::string mHostname; @@ -392,6 +392,7 @@ public: { mTime = current_time; } + virtual ~LLCertValidationExpirationException() throw() {} LLDate GetTime() { return mTime; } protected: LLDate mTime; @@ -403,6 +404,7 @@ public: LLCertKeyUsageValidationException(LLPointer cert) : LLCertException(cert, "CertKeyUsage") { } + virtual ~LLCertKeyUsageValidationException() throw() {} protected: }; @@ -412,6 +414,7 @@ public: LLCertBasicConstraintsValidationException(LLPointer cert) : LLCertException(cert, "CertBasicConstraints") { } + virtual ~LLCertBasicConstraintsValidationException() throw() {} protected: }; @@ -421,6 +424,7 @@ public: LLCertValidationInvalidSignatureException(LLPointer cert) : LLCertException(cert, "CertInvalidSignature") { } + virtual ~LLCertValidationInvalidSignatureException() throw() {} protected: }; -- cgit v1.2.3 From 2f003fd5a99f9b9f42b74a70d3a53e818b9bcee4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 13 Jul 2016 14:58:12 -0400 Subject: MAINT-5011: Throw an actual exception in Force Software Exception. http://en.cppreference.com/w/cpp/language/throw says of the plain throw syntax: "This form is only allowed when an exception is presently being handled (it calls std::terminate if used otherwise)." On advice from Oz, replace plain 'throw;' with throwing a std::runtime_error. --- indra/newview/llappviewer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 604e45f314..a812a5e518 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -231,6 +231,7 @@ #include "llcoproceduremanager.h" #include "llviewereventrecorder.h" +#include // *FIX: These extern globals should be cleaned up. // The globals either represent state/config/resource-storage of either @@ -5512,8 +5513,7 @@ void LLAppViewer::forceErrorInfiniteLoop() void LLAppViewer::forceErrorSoftwareException() { LL_WARNS() << "Forcing a deliberate exception" << LL_ENDL; - // *FIX: Any way to insure it won't be handled? - throw; + throw std::runtime_error("User selected Force Software Exception"); } void LLAppViewer::forceErrorDriverCrash() -- cgit v1.2.3 From cefa598e49490bfedb86a5be0747ec3c856d06bf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 14 Jul 2016 11:33:29 -0400 Subject: MAINT-5011: Per NickyD, make LLCertException::getMessage() const. Also getCert(). Also LLProtectedDataException::getMessage(). --- indra/newview/llsecapi.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llsecapi.h b/indra/newview/llsecapi.h index 02438f77b7..55c6d95cd8 100644 --- a/indra/newview/llsecapi.h +++ b/indra/newview/llsecapi.h @@ -124,7 +124,7 @@ struct LLProtectedDataException: public std::runtime_error { LL_WARNS("SECAPI") << "Protected Data Error: " << msg << LL_ENDL; } - std::string getMessage() { return what(); } + std::string getMessage() const { return what(); } }; // class LLCertificate @@ -344,8 +344,8 @@ public: LL_WARNS("SECAPI") << "Certificate Error: " << msg << LL_ENDL; } virtual ~LLCertException() throw() {} - LLPointer getCert() { return mCert; } - std::string getMessage() { return what(); } + LLPointer getCert() const { return mCert; } + std::string getMessage() const { return what(); } protected: LLPointer mCert; }; -- cgit v1.2.3 From 47d93e4f65493977217cfed53ff68eb926cf9bb7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 19 Jul 2016 14:08:43 -0400 Subject: DRTVWR-418: Remove rogue getMessage() from llsecapi.h exceptions. The LLProtectedDataException and LLCertException exception classes didn't used to be derived from std::exception, so they followed their own getMessage() convention instead of the standard what() convention. Now that they're derived from std::exception, remove getMessage() and change its few consumers to use what() instead. Thanks NickyD for suggesting. --- indra/newview/llappcorehttp.cpp | 4 ++-- indra/newview/llsecapi.cpp | 2 +- indra/newview/llsecapi.h | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappcorehttp.cpp b/indra/newview/llappcorehttp.cpp index 49291ea564..dbcae57de7 100644 --- a/indra/newview/llappcorehttp.cpp +++ b/indra/newview/llappcorehttp.cpp @@ -534,7 +534,7 @@ LLCore::HttpStatus LLAppCoreHttp::sslVerify(const std::string &url, // somewhat clumsy, as we may run into errors that do not map directly to curl // error codes. Should be refactored with login refactoring, perhaps. result = LLCore::HttpStatus(LLCore::HttpStatus::EXT_CURL_EASY, CURLE_SSL_CACERT); - result.setMessage(cert_exception.getMessage()); + result.setMessage(cert_exception.what()); LLPointer cert = cert_exception.getCert(); cert->ref(); // adding an extra ref here result.setErrorData(cert.get()); @@ -544,7 +544,7 @@ LLCore::HttpStatus LLAppCoreHttp::sslVerify(const std::string &url, catch (LLCertException &cert_exception) { result = LLCore::HttpStatus(LLCore::HttpStatus::EXT_CURL_EASY, CURLE_SSL_PEER_CERTIFICATE); - result.setMessage(cert_exception.getMessage()); + result.setMessage(cert_exception.what()); LLPointer cert = cert_exception.getCert(); cert->ref(); // adding an extra ref here result.setErrorData(cert.get()); diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index c27709a57b..e871570786 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -64,7 +64,7 @@ void initializeSecHandler() } catch (LLProtectedDataException e) { - exception_msg = e.getMessage(); + exception_msg = e.what(); } } if (!exception_msg.empty()) // an exception was thrown. diff --git a/indra/newview/llsecapi.h b/indra/newview/llsecapi.h index 55c6d95cd8..535a112638 100644 --- a/indra/newview/llsecapi.h +++ b/indra/newview/llsecapi.h @@ -124,7 +124,6 @@ struct LLProtectedDataException: public std::runtime_error { LL_WARNS("SECAPI") << "Protected Data Error: " << msg << LL_ENDL; } - std::string getMessage() const { return what(); } }; // class LLCertificate @@ -345,7 +344,6 @@ public: } virtual ~LLCertException() throw() {} LLPointer getCert() const { return mCert; } - std::string getMessage() const { return what(); } protected: LLPointer mCert; }; -- cgit v1.2.3 From 9c49a6c91dd9b5bbe811fcd91d8992ed6bac33e7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 19 Jul 2016 16:25:25 -0400 Subject: MAINT-5011: Introduce LLException base class for viewer exceptions. This also introduces LLContinueError for exceptions which should interrupt some part of viewer processing (e.g. the current coroutine) but should attempt to let the viewer session proceed. Derive all existing viewer exception classes from LLException rather than from std::runtime_error or std::logic_error. Use BOOST_THROW_EXCEPTION() rather than plain 'throw' to enrich the thrown exception with source file, line number and containing function. --- indra/newview/llappviewer.cpp | 6 ++-- indra/newview/llcommandlineparser.cpp | 25 +++++++++-------- indra/newview/llsecapi.cpp | 3 +- indra/newview/llsecapi.h | 10 +++---- indra/newview/llsechandler_basic.cpp | 53 ++++++++++++++++++----------------- 5 files changed, 51 insertions(+), 46 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index a812a5e518..fdef556589 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -122,6 +122,7 @@ #include "llleap.h" #include "stringize.h" #include "llcoros.h" +#include "llexception.h" #if !LL_LINUX #include "cef/llceflib.h" #endif @@ -131,6 +132,7 @@ #include #include #include +#include #if LL_WINDOWS # include // For _SH_DENYWR in processMarkerFiles @@ -231,8 +233,6 @@ #include "llcoproceduremanager.h" #include "llviewereventrecorder.h" -#include - // *FIX: These extern globals should be cleaned up. // The globals either represent state/config/resource-storage of either // this app, or another 'component' of the viewer. App globals should be @@ -5513,7 +5513,7 @@ void LLAppViewer::forceErrorInfiniteLoop() void LLAppViewer::forceErrorSoftwareException() { LL_WARNS() << "Forcing a deliberate exception" << LL_ENDL; - throw std::runtime_error("User selected Force Software Exception"); + BOOST_THROW_EXCEPTION(LLException("User selected Force Software Exception")); } void LLAppViewer::forceErrorDriverCrash() diff --git a/indra/newview/llcommandlineparser.cpp b/indra/newview/llcommandlineparser.cpp index 1819fc74ee..54f96b8872 100644 --- a/indra/newview/llcommandlineparser.cpp +++ b/indra/newview/llcommandlineparser.cpp @@ -26,6 +26,7 @@ #include "llviewerprecompiledheaders.h" #include "llcommandlineparser.h" +#include "llexception.h" // *NOTE: The boost::lexical_cast generates // the warning C4701(local used with out assignment) in VC7.1. @@ -42,6 +43,7 @@ #include #include #include +#include #if _MSC_VER # pragma warning(pop) @@ -98,14 +100,14 @@ namespace bool gPastLastOption = false; } -class LLCLPError : public std::logic_error { +class LLCLPError : public LLException { public: - LLCLPError(const std::string& what) : std::logic_error(what) {} + LLCLPError(const std::string& what) : LLException(what) {} }; -class LLCLPLastOption : public std::logic_error { +class LLCLPLastOption : public LLException { public: - LLCLPLastOption(const std::string& what) : std::logic_error(what) {} + LLCLPLastOption(const std::string& what) : LLException(what) {} }; class LLCLPValue : public po::value_semantic_codecvt_helper @@ -202,17 +204,17 @@ protected: { if(gPastLastOption) { - throw(LLCLPLastOption("Don't parse no more!")); + BOOST_THROW_EXCEPTION(LLCLPLastOption("Don't parse no more!")); } // Error checks. Needed? if (!value_store.empty() && !is_composing()) { - throw(LLCLPError("Non composing value with multiple occurences.")); + BOOST_THROW_EXCEPTION(LLCLPError("Non composing value with multiple occurences.")); } if (new_tokens.size() < min_tokens() || new_tokens.size() > max_tokens()) { - throw(LLCLPError("Illegal number of tokens specified.")); + BOOST_THROW_EXCEPTION(LLCLPError("Illegal number of tokens specified.")); } if(value_store.empty()) @@ -466,7 +468,7 @@ onevalue(const std::string& option, { // What does it mean when the user specifies a command-line switch // that requires a value, but omits the value? Complain. - throw LLCLPError(STRINGIZE("No value specified for --" << option << "!")); + BOOST_THROW_EXCEPTION(LLCLPError(STRINGIZE("No value specified for --" << option << "!"))); } else if (value.size() > 1) { @@ -484,9 +486,10 @@ void badvalue(const std::string& option, // If the user passes an unusable value for a command-line switch, it // seems like a really bad idea to just ignore it, even with a log // warning. - throw LLCLPError(STRINGIZE("Invalid value specified by command-line switch '" << option - << "' for variable '" << varname << "' of type " << type - << ": '" << value << "'")); + BOOST_THROW_EXCEPTION( + LLCLPError(STRINGIZE("Invalid value specified by command-line switch '" << option + << "' for variable '" << varname << "' of type " << type + << ": '" << value << "'"))); } template diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index e871570786..bcb9417820 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -29,6 +29,7 @@ #include "llviewerprecompiledheaders.h" #include "llsecapi.h" #include "llsechandler_basic.h" +#include #include #include #include @@ -69,7 +70,7 @@ void initializeSecHandler() } if (!exception_msg.empty()) // an exception was thrown. { - throw LLProtectedDataException(exception_msg); + BOOST_THROW_EXCEPTION(LLProtectedDataException(exception_msg)); } } diff --git a/indra/newview/llsecapi.h b/indra/newview/llsecapi.h index 535a112638..6af5a28fa5 100644 --- a/indra/newview/llsecapi.h +++ b/indra/newview/llsecapi.h @@ -32,7 +32,7 @@ #include #include #include "llpointer.h" -#include +#include "llexception.h" #ifdef LL_WINDOWS #pragma warning(disable:4250) @@ -117,10 +117,10 @@ -struct LLProtectedDataException: public std::runtime_error +struct LLProtectedDataException: public LLException { LLProtectedDataException(const std::string& msg): - std::runtime_error(msg) + LLException(msg) { LL_WARNS("SECAPI") << "Protected Data Error: " << msg << LL_ENDL; } @@ -331,11 +331,11 @@ std::ostream& operator <<(std::ostream& s, const LLCredential& cred); // All error handling is via exceptions. -class LLCertException: public std::runtime_error +class LLCertException: public LLException { public: LLCertException(LLPointer cert, const std::string& msg): - std::runtime_error(msg) + LLException(msg) { mCert = cert; diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 40516f9bbb..39ce64ad0e 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -35,6 +35,7 @@ #include "llfile.h" #include "lldir.h" #include "llviewercontrol.h" +#include #include #include #include @@ -72,14 +73,14 @@ LLBasicCertificate::LLBasicCertificate(const std::string& pem_cert) if(pem_bio == NULL) { LL_WARNS("SECAPI") << "Could not allocate an openssl memory BIO." << LL_ENDL; - throw LLInvalidCertificate(this); + BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); } mCert = NULL; PEM_read_bio_X509(pem_bio, &mCert, 0, NULL); BIO_free(pem_bio); if (!mCert) { - throw LLInvalidCertificate(this); + BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); } } @@ -88,7 +89,7 @@ LLBasicCertificate::LLBasicCertificate(X509* pCert) { if (!pCert || !pCert->cert_info) { - throw LLInvalidCertificate(this); + BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); } mCert = X509_dup(pCert); } @@ -873,22 +874,22 @@ void _validateCert(int validation_policy, // check basic properties exist in the cert if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info.has(CERT_SUBJECT_NAME_STRING)) { - throw LLCertException(cert, "Cert doesn't have a Subject Name"); + BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have a Subject Name")); } if(!current_cert_info.has(CERT_ISSUER_NAME_STRING)) { - throw LLCertException(cert, "Cert doesn't have an Issuer Name"); + BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an Issuer Name")); } // check basic properties exist in the cert if(!current_cert_info.has(CERT_VALID_FROM) || !current_cert_info.has(CERT_VALID_TO)) { - throw LLCertException(cert, "Cert doesn't have an expiration period"); + BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an expiration period")); } if (!current_cert_info.has(CERT_SHA1_DIGEST)) { - throw LLCertException(cert, "No SHA1 digest"); + BOOST_THROW_EXCEPTION(LLCertException(cert, "No SHA1 digest")); } if (validation_policy & VALIDATION_POLICY_TIME) @@ -903,7 +904,7 @@ void _validateCert(int validation_policy, if((validation_date < current_cert_info[CERT_VALID_FROM].asDate()) || (validation_date > current_cert_info[CERT_VALID_TO].asDate())) { - throw LLCertValidationExpirationException(cert, validation_date); + BOOST_THROW_EXCEPTION(LLCertValidationExpirationException(cert, validation_date)); } } if (validation_policy & VALIDATION_POLICY_SSL_KU) @@ -914,14 +915,14 @@ void _validateCert(int validation_policy, !(_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], LLSD((std::string)CERT_KU_KEY_ENCIPHERMENT))))) { - throw LLCertKeyUsageValidationException(cert); + BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); } // only validate EKU if the cert has it if(current_cert_info.has(CERT_EXTENDED_KEY_USAGE) && current_cert_info[CERT_EXTENDED_KEY_USAGE].isArray() && (!_LLSDArrayIncludesValue(current_cert_info[CERT_EXTENDED_KEY_USAGE], LLSD((std::string)CERT_EKU_SERVER_AUTH)))) { - throw LLCertKeyUsageValidationException(cert); + BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); } } if (validation_policy & VALIDATION_POLICY_CA_KU) @@ -930,7 +931,7 @@ void _validateCert(int validation_policy, (!_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], (std::string)CERT_KU_CERT_SIGN))) { - throw LLCertKeyUsageValidationException(cert); + BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); } } @@ -942,13 +943,13 @@ void _validateCert(int validation_policy, if(!current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_CA) || !current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_CA]) { - throw LLCertBasicConstraintsValidationException(cert); + BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); } if (current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_PATHLEN) && ((current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger() != 0) && (depth > current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger()))) { - throw LLCertBasicConstraintsValidationException(cert); + BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); } } } @@ -1018,7 +1019,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(cert_chain->size() < 1) { - throw LLCertException(NULL, "No certs in chain"); + BOOST_THROW_EXCEPTION(LLCertException(NULL, "No certs in chain")); } iterator current_cert = cert_chain->begin(); LLSD current_cert_info; @@ -1033,11 +1034,11 @@ void LLBasicCertificateStore::validate(int validation_policy, (*current_cert)->getLLSD(current_cert_info); if(!validation_params.has(CERT_HOSTNAME)) { - throw LLCertException((*current_cert), "No hostname passed in for validation"); + BOOST_THROW_EXCEPTION(LLCertException((*current_cert), "No hostname passed in for validation")); } if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info[CERT_SUBJECT_NAME].has(CERT_NAME_CN)) { - throw LLInvalidCertificate((*current_cert)); + BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); } LL_DEBUGS("SECAPI") << "Validating the hostname " << validation_params[CERT_HOSTNAME].asString() << @@ -1054,7 +1055,7 @@ void LLBasicCertificateStore::validate(int validation_policy, X509* cert_x509 = (*current_cert)->getOpenSSLX509(); if(!cert_x509) { - throw LLInvalidCertificate((*current_cert)); + BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); } std::string sha1_hash((const char *)cert_x509->sha1_hash, SHA_DIGEST_LENGTH); X509_free( cert_x509 ); @@ -1075,7 +1076,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if((validation_date < cache_entry->second.first) || (validation_date > cache_entry->second.second)) { - throw LLCertValidationExpirationException((*current_cert), validation_date); + BOOST_THROW_EXCEPTION(LLCertValidationExpirationException((*current_cert), validation_date)); } } // successfully found in cache @@ -1107,7 +1108,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*current_cert), previous_cert)) { - throw LLCertValidationInvalidSignatureException(previous_cert); + BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(previous_cert)); } } _validateCert(local_validation_policy, @@ -1156,7 +1157,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*found_store_cert), (*current_cert))) { - throw LLCertValidationInvalidSignatureException(*current_cert); + BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(*current_cert)); } // successfully validated. mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1173,7 +1174,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if (validation_policy & VALIDATION_POLICY_TRUSTED) { // we reached the end without finding a trusted cert. - throw LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1]); + BOOST_THROW_EXCEPTION(LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1])); } mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1261,7 +1262,7 @@ void LLSecAPIBasicHandler::_readProtectedData() protected_data_stream.read((char *)salt, STORE_SALT_SIZE); if (protected_data_stream.gcount() < STORE_SALT_SIZE) { - throw LLProtectedDataException("Config file too short."); + BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file too short.")); } cipher.decrypt(salt, STORE_SALT_SIZE); @@ -1301,7 +1302,7 @@ void LLSecAPIBasicHandler::_readProtectedData() if (parser->parse(parse_stream, mProtectedDataMap, LLSDSerialize::SIZE_UNLIMITED) == LLSDParser::PARSE_FAILURE) { - throw LLProtectedDataException("Config file cannot be decrypted."); + BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file cannot be decrypted.")); } } } @@ -1372,7 +1373,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //throw LLProtectedDataException("Error writing Protected Data Store"); + //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); } try @@ -1387,7 +1388,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //throw LLProtectedDataException("Could not overwrite protected data store"); + //BOOST_THROW_EXCEPTION(LLProtectedDataException("Could not overwrite protected data store")); } } catch (...) @@ -1401,7 +1402,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() //crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //throw LLProtectedDataException("Error writing Protected Data Store"); + //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); } } -- cgit v1.2.3 From 5e9d2f57c82a57307a48afea09aa539b9fa80abf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 11:36:24 -0400 Subject: MAINT-5011: Use LLTHROW() instead of plain BOOST_THROW_EXCEPTION(). A level of preprocessor indirection lets us later change the implementation if desired. --- indra/newview/llappviewer.cpp | 3 +- indra/newview/llcommandlineparser.cpp | 17 ++++++----- indra/newview/llsecapi.cpp | 4 +-- indra/newview/llsechandler_basic.cpp | 54 +++++++++++++++++------------------ 4 files changed, 38 insertions(+), 40 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index fdef556589..0eca63da46 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -132,7 +132,6 @@ #include #include #include -#include #if LL_WINDOWS # include // For _SH_DENYWR in processMarkerFiles @@ -5513,7 +5512,7 @@ void LLAppViewer::forceErrorInfiniteLoop() void LLAppViewer::forceErrorSoftwareException() { LL_WARNS() << "Forcing a deliberate exception" << LL_ENDL; - BOOST_THROW_EXCEPTION(LLException("User selected Force Software Exception")); + LLTHROW(LLException("User selected Force Software Exception")); } void LLAppViewer::forceErrorDriverCrash() diff --git a/indra/newview/llcommandlineparser.cpp b/indra/newview/llcommandlineparser.cpp index 54f96b8872..90a5483dc9 100644 --- a/indra/newview/llcommandlineparser.cpp +++ b/indra/newview/llcommandlineparser.cpp @@ -43,7 +43,6 @@ #include #include #include -#include #if _MSC_VER # pragma warning(pop) @@ -52,6 +51,7 @@ #include "llsdserialize.h" #include "llerror.h" #include "stringize.h" +#include "llexception.h" #include #include #include @@ -204,17 +204,17 @@ protected: { if(gPastLastOption) { - BOOST_THROW_EXCEPTION(LLCLPLastOption("Don't parse no more!")); + LLTHROW(LLCLPLastOption("Don't parse no more!")); } // Error checks. Needed? if (!value_store.empty() && !is_composing()) { - BOOST_THROW_EXCEPTION(LLCLPError("Non composing value with multiple occurences.")); + LLTHROW(LLCLPError("Non composing value with multiple occurences.")); } if (new_tokens.size() < min_tokens() || new_tokens.size() > max_tokens()) { - BOOST_THROW_EXCEPTION(LLCLPError("Illegal number of tokens specified.")); + LLTHROW(LLCLPError("Illegal number of tokens specified.")); } if(value_store.empty()) @@ -468,7 +468,7 @@ onevalue(const std::string& option, { // What does it mean when the user specifies a command-line switch // that requires a value, but omits the value? Complain. - BOOST_THROW_EXCEPTION(LLCLPError(STRINGIZE("No value specified for --" << option << "!"))); + LLTHROW(LLCLPError(STRINGIZE("No value specified for --" << option << "!"))); } else if (value.size() > 1) { @@ -486,10 +486,9 @@ void badvalue(const std::string& option, // If the user passes an unusable value for a command-line switch, it // seems like a really bad idea to just ignore it, even with a log // warning. - BOOST_THROW_EXCEPTION( - LLCLPError(STRINGIZE("Invalid value specified by command-line switch '" << option - << "' for variable '" << varname << "' of type " << type - << ": '" << value << "'"))); + LLTHROW(LLCLPError(STRINGIZE("Invalid value specified by command-line switch '" << option + << "' for variable '" << varname << "' of type " << type + << ": '" << value << "'"))); } template diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index bcb9417820..933f7bb16e 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -29,7 +29,7 @@ #include "llviewerprecompiledheaders.h" #include "llsecapi.h" #include "llsechandler_basic.h" -#include +#include "llexception.h" #include #include #include @@ -70,7 +70,7 @@ void initializeSecHandler() } if (!exception_msg.empty()) // an exception was thrown. { - BOOST_THROW_EXCEPTION(LLProtectedDataException(exception_msg)); + LLTHROW(LLProtectedDataException(exception_msg)); } } diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 39ce64ad0e..183a625382 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -35,7 +35,7 @@ #include "llfile.h" #include "lldir.h" #include "llviewercontrol.h" -#include +#include "llexception.h" #include #include #include @@ -73,14 +73,14 @@ LLBasicCertificate::LLBasicCertificate(const std::string& pem_cert) if(pem_bio == NULL) { LL_WARNS("SECAPI") << "Could not allocate an openssl memory BIO." << LL_ENDL; - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } mCert = NULL; PEM_read_bio_X509(pem_bio, &mCert, 0, NULL); BIO_free(pem_bio); if (!mCert) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } } @@ -89,7 +89,7 @@ LLBasicCertificate::LLBasicCertificate(X509* pCert) { if (!pCert || !pCert->cert_info) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate(this)); + LLTHROW(LLInvalidCertificate(this)); } mCert = X509_dup(pCert); } @@ -874,22 +874,22 @@ void _validateCert(int validation_policy, // check basic properties exist in the cert if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info.has(CERT_SUBJECT_NAME_STRING)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have a Subject Name")); + LLTHROW(LLCertException(cert, "Cert doesn't have a Subject Name")); } if(!current_cert_info.has(CERT_ISSUER_NAME_STRING)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an Issuer Name")); + LLTHROW(LLCertException(cert, "Cert doesn't have an Issuer Name")); } // check basic properties exist in the cert if(!current_cert_info.has(CERT_VALID_FROM) || !current_cert_info.has(CERT_VALID_TO)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "Cert doesn't have an expiration period")); + LLTHROW(LLCertException(cert, "Cert doesn't have an expiration period")); } if (!current_cert_info.has(CERT_SHA1_DIGEST)) { - BOOST_THROW_EXCEPTION(LLCertException(cert, "No SHA1 digest")); + LLTHROW(LLCertException(cert, "No SHA1 digest")); } if (validation_policy & VALIDATION_POLICY_TIME) @@ -904,7 +904,7 @@ void _validateCert(int validation_policy, if((validation_date < current_cert_info[CERT_VALID_FROM].asDate()) || (validation_date > current_cert_info[CERT_VALID_TO].asDate())) { - BOOST_THROW_EXCEPTION(LLCertValidationExpirationException(cert, validation_date)); + LLTHROW(LLCertValidationExpirationException(cert, validation_date)); } } if (validation_policy & VALIDATION_POLICY_SSL_KU) @@ -915,14 +915,14 @@ void _validateCert(int validation_policy, !(_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], LLSD((std::string)CERT_KU_KEY_ENCIPHERMENT))))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } // only validate EKU if the cert has it if(current_cert_info.has(CERT_EXTENDED_KEY_USAGE) && current_cert_info[CERT_EXTENDED_KEY_USAGE].isArray() && (!_LLSDArrayIncludesValue(current_cert_info[CERT_EXTENDED_KEY_USAGE], LLSD((std::string)CERT_EKU_SERVER_AUTH)))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } } if (validation_policy & VALIDATION_POLICY_CA_KU) @@ -931,7 +931,7 @@ void _validateCert(int validation_policy, (!_LLSDArrayIncludesValue(current_cert_info[CERT_KEY_USAGE], (std::string)CERT_KU_CERT_SIGN))) { - BOOST_THROW_EXCEPTION(LLCertKeyUsageValidationException(cert)); + LLTHROW(LLCertKeyUsageValidationException(cert)); } } @@ -943,13 +943,13 @@ void _validateCert(int validation_policy, if(!current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_CA) || !current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_CA]) { - BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); + LLTHROW(LLCertBasicConstraintsValidationException(cert)); } if (current_cert_info[CERT_BASIC_CONSTRAINTS].has(CERT_BASIC_CONSTRAINTS_PATHLEN) && ((current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger() != 0) && (depth > current_cert_info[CERT_BASIC_CONSTRAINTS][CERT_BASIC_CONSTRAINTS_PATHLEN].asInteger()))) { - BOOST_THROW_EXCEPTION(LLCertBasicConstraintsValidationException(cert)); + LLTHROW(LLCertBasicConstraintsValidationException(cert)); } } } @@ -1019,7 +1019,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(cert_chain->size() < 1) { - BOOST_THROW_EXCEPTION(LLCertException(NULL, "No certs in chain")); + LLTHROW(LLCertException(NULL, "No certs in chain")); } iterator current_cert = cert_chain->begin(); LLSD current_cert_info; @@ -1034,11 +1034,11 @@ void LLBasicCertificateStore::validate(int validation_policy, (*current_cert)->getLLSD(current_cert_info); if(!validation_params.has(CERT_HOSTNAME)) { - BOOST_THROW_EXCEPTION(LLCertException((*current_cert), "No hostname passed in for validation")); + LLTHROW(LLCertException((*current_cert), "No hostname passed in for validation")); } if(!current_cert_info.has(CERT_SUBJECT_NAME) || !current_cert_info[CERT_SUBJECT_NAME].has(CERT_NAME_CN)) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); + LLTHROW(LLInvalidCertificate((*current_cert))); } LL_DEBUGS("SECAPI") << "Validating the hostname " << validation_params[CERT_HOSTNAME].asString() << @@ -1055,7 +1055,7 @@ void LLBasicCertificateStore::validate(int validation_policy, X509* cert_x509 = (*current_cert)->getOpenSSLX509(); if(!cert_x509) { - BOOST_THROW_EXCEPTION(LLInvalidCertificate((*current_cert))); + LLTHROW(LLInvalidCertificate((*current_cert))); } std::string sha1_hash((const char *)cert_x509->sha1_hash, SHA_DIGEST_LENGTH); X509_free( cert_x509 ); @@ -1076,7 +1076,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if((validation_date < cache_entry->second.first) || (validation_date > cache_entry->second.second)) { - BOOST_THROW_EXCEPTION(LLCertValidationExpirationException((*current_cert), validation_date)); + LLTHROW(LLCertValidationExpirationException((*current_cert), validation_date)); } } // successfully found in cache @@ -1108,7 +1108,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*current_cert), previous_cert)) { - BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(previous_cert)); + LLTHROW(LLCertValidationInvalidSignatureException(previous_cert)); } } _validateCert(local_validation_policy, @@ -1157,7 +1157,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if(!_verify_signature((*found_store_cert), (*current_cert))) { - BOOST_THROW_EXCEPTION(LLCertValidationInvalidSignatureException(*current_cert)); + LLTHROW(LLCertValidationInvalidSignatureException(*current_cert)); } // successfully validated. mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1174,7 +1174,7 @@ void LLBasicCertificateStore::validate(int validation_policy, if (validation_policy & VALIDATION_POLICY_TRUSTED) { // we reached the end without finding a trusted cert. - BOOST_THROW_EXCEPTION(LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1])); + LLTHROW(LLCertValidationTrustException((*cert_chain)[cert_chain->size()-1])); } mTrustedCertCache[sha1_hash] = std::pair(from_time, to_time); @@ -1262,7 +1262,7 @@ void LLSecAPIBasicHandler::_readProtectedData() protected_data_stream.read((char *)salt, STORE_SALT_SIZE); if (protected_data_stream.gcount() < STORE_SALT_SIZE) { - BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file too short.")); + LLTHROW(LLProtectedDataException("Config file too short.")); } cipher.decrypt(salt, STORE_SALT_SIZE); @@ -1302,7 +1302,7 @@ void LLSecAPIBasicHandler::_readProtectedData() if (parser->parse(parse_stream, mProtectedDataMap, LLSDSerialize::SIZE_UNLIMITED) == LLSDParser::PARSE_FAILURE) { - BOOST_THROW_EXCEPTION(LLProtectedDataException("Config file cannot be decrypted.")); + LLTHROW(LLProtectedDataException("Config file cannot be decrypted.")); } } } @@ -1373,7 +1373,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); + //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } try @@ -1388,7 +1388,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Could not overwrite protected data store")); + //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); } } catch (...) @@ -1402,7 +1402,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() //crash in LLSecAPIBasicHandler::_writeProtectedData() // Decided throwing an exception here was overkill until we figure out why this happens - //BOOST_THROW_EXCEPTION(LLProtectedDataException("Error writing Protected Data Store")); + //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } } -- cgit v1.2.3 From 993f54f6e91d78a9c2e1389ad878d6bd46e9be5b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 15:40:03 -0400 Subject: MAINT-5011: Try to enrich catch (...) logging throughout viewer. Turns out we have a surprising number of catch (...) clauses in the viewer code base. If all we currently do is LL_ERRS() << "unknown exception" << LL_ENDL; then call CRASH_ON_UNHANDLED_EXCEPTION() instead. If what we do is LL_WARNS() << "unknown exception" << LL_ENDL; then call LOG_UNHANDLED_EXCEPTION() instead. Since many places need LOG_UNHANDLED_EXCEPTION() and nobody catches LLContinueError yet, eliminate LLContinueError& parameter from LOG_UNHANDLED_EXCEPTION(). This permits us to use the same log message as CRASH_ON_UNHANDLED_EXCEPTION(), just with a different severity level. Where a catch (...) clause actually provides contextual information, or makes an error string, add boost::current_exception_diagnostic_information() to try to figure out actual exception class and message. --- indra/newview/llaccountingcostmanager.cpp | 3 ++- indra/newview/llappcorehttp.cpp | 2 ++ indra/newview/llsecapi.cpp | 1 + indra/newview/llsechandler_basic.cpp | 10 +++++++--- 4 files changed, 12 insertions(+), 4 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llaccountingcostmanager.cpp b/indra/newview/llaccountingcostmanager.cpp index 92a5413adb..a4a54cf8fb 100644 --- a/indra/newview/llaccountingcostmanager.cpp +++ b/indra/newview/llaccountingcostmanager.cpp @@ -31,6 +31,7 @@ #include "llcoros.h" #include "lleventcoro.h" #include "llcorehttputil.h" +#include "llexception.h" #include #include @@ -160,7 +161,7 @@ void LLAccountingCostManager::accountingCostCoro(std::string url, } catch (...) { - LL_WARNS() << "Caught unknown exception." << LL_ENDL; + LOG_UNHANDLED_EXCEPTION(); } mPendingObjectQuota.clear(); diff --git a/indra/newview/llappcorehttp.cpp b/indra/newview/llappcorehttp.cpp index dbcae57de7..7ec041c02e 100644 --- a/indra/newview/llappcorehttp.cpp +++ b/indra/newview/llappcorehttp.cpp @@ -30,6 +30,7 @@ #include "llappviewer.h" #include "llviewercontrol.h" +#include "llexception.h" #include #include @@ -551,6 +552,7 @@ LLCore::HttpStatus LLAppCoreHttp::sslVerify(const std::string &url, } catch (...) { + LOG_UNHANDLED_EXCEPTION(); // any other odd error, we just handle as a connect error. result = LLCore::HttpStatus(LLCore::HttpStatus::EXT_CURL_EASY, CURLE_SSL_CONNECT_ERROR); } diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index 933f7bb16e..6ca1e63417 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -121,6 +121,7 @@ LLSD LLCredential::getLoginParams() } catch (...) { + LOG_UNHANDLED_EXCEPTION(); // we could have corrupt data, so simply return a null login param if so LL_WARNS("AppInit") << "Invalid credential" << LL_ENDL; } diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 183a625382..54396cb9a4 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -38,6 +38,7 @@ #include "llexception.h" #include #include +#include #include #include #include @@ -618,7 +619,8 @@ void LLBasicCertificateStore::load_from_file(const std::string& filename) } catch (...) { - LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file." << LL_ENDL; + LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file: " + << boost::current_exception_diagnostic_information() << LL_ENDL; } X509_free(cert_x509); cert_x509 = NULL; @@ -1365,7 +1367,8 @@ void LLSecAPIBasicHandler::_writeProtectedData() } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store)" << LL_ENDL; + LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store): " + << boost::current_exception_diagnostic_information() << LL_ENDL; // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. @@ -1394,7 +1397,8 @@ void LLSecAPIBasicHandler::_writeProtectedData() catch (...) { LL_WARNS() << "LLProtectedDataException(Error renaming '" << tmp_filename - << "' to '" << mProtectedDataFilename << "')" << LL_ENDL; + << "' to '" << mProtectedDataFilename << "'): " + << boost::current_exception_diagnostic_information() << LL_ENDL; // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. -- cgit v1.2.3 From 4d10172d8b2c72fa809e322a3b4ff326b19ff340 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Aug 2016 17:33:44 -0400 Subject: MAINT-5011: Catch unhandled exceptions in LLCoros coroutines. Wrap coroutine call in try/catch in top-level coroutine wrapper function LLCoros::toplevel(). Distinguish exception classes derived from LLContinueError (log and continue) from all others (crash with LL_ERRS). Enhance CRASH_ON_UNHANDLED_EXCEPTIONS() and LOG_UNHANDLED_EXCEPTIONS() macros to accept a context string to supplement the log message. This lets us replace many places that called boost::current_exception_diagnostic_information() with LOG_UNHANDLED_EXCEPTIONS() instead, since the explicit calls were mostly to log supplemental information. Provide supplemental information (coroutine name, function parameters) for some of the previous LOG_UNHANDLED_EXCEPTIONS() calls. This information duplicates LL_DEBUGS() information at the top of these functions, but in a typical log file we wouldn't see the LL_DEBUGS() message. Eliminate a few catch (std::exception e) clauses: the information we get from boost::current_exception_diagnostic_information() in a catch (...) clause makes it unnecessary to distinguish. In a few cases, add a final 'throw;' to a catch (...) clause: having logged the local context info, propagate the exception to be caught by higher-level try/catch. In a couple places, couldn't resist reconciling indentation within a particular function: tabs where the rest of the function uses tabs, spaces where the rest of the function uses spaces. In LLLogin::Impl::loginCoro(), eliminate some confusing comments about an array of rewritten URIs that date back to a long-deleted implementation. --- indra/newview/llaccountingcostmanager.cpp | 9 +++---- indra/newview/llappcorehttp.cpp | 3 ++- indra/newview/llsecapi.cpp | 9 ++++--- indra/newview/llsechandler_basic.cpp | 43 ++++++++++++++----------------- 4 files changed, 32 insertions(+), 32 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llaccountingcostmanager.cpp b/indra/newview/llaccountingcostmanager.cpp index a4a54cf8fb..1dddf52961 100644 --- a/indra/newview/llaccountingcostmanager.cpp +++ b/indra/newview/llaccountingcostmanager.cpp @@ -32,6 +32,7 @@ #include "lleventcoro.h" #include "llcorehttputil.h" #include "llexception.h" +#include "stringize.h" #include #include @@ -155,13 +156,11 @@ void LLAccountingCostManager::accountingCostCoro(std::string url, } while (false); } - catch (std::exception e) - { - LL_WARNS() << "Caught exception '" << e.what() << "'" << LL_ENDL; - } catch (...) { - LOG_UNHANDLED_EXCEPTION(); + LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << LLCoros::instance().getName() + << "('" << url << "')")); + throw; } mPendingObjectQuota.clear(); diff --git a/indra/newview/llappcorehttp.cpp b/indra/newview/llappcorehttp.cpp index 7ec041c02e..c1f898284a 100644 --- a/indra/newview/llappcorehttp.cpp +++ b/indra/newview/llappcorehttp.cpp @@ -31,6 +31,7 @@ #include "llappviewer.h" #include "llviewercontrol.h" #include "llexception.h" +#include "stringize.h" #include #include @@ -552,7 +553,7 @@ LLCore::HttpStatus LLAppCoreHttp::sslVerify(const std::string &url, } catch (...) { - LOG_UNHANDLED_EXCEPTION(); + LOG_UNHANDLED_EXCEPTION(STRINGIZE("('" << url << "')")); // any other odd error, we just handle as a connect error. result = LLCore::HttpStatus(LLCore::HttpStatus::EXT_CURL_EASY, CURLE_SSL_CONNECT_ERROR); } diff --git a/indra/newview/llsecapi.cpp b/indra/newview/llsecapi.cpp index 6ca1e63417..72d7cf1e45 100644 --- a/indra/newview/llsecapi.cpp +++ b/indra/newview/llsecapi.cpp @@ -30,6 +30,7 @@ #include "llsecapi.h" #include "llsechandler_basic.h" #include "llexception.h" +#include "stringize.h" #include #include #include @@ -102,6 +103,7 @@ std::ostream& operator <<(std::ostream& s, const LLCredential& cred) LLSD LLCredential::getLoginParams() { LLSD result = LLSD::emptyMap(); + std::string username; try { if (mIdentifier["type"].asString() == "agent") @@ -110,18 +112,19 @@ LLSD LLCredential::getLoginParams() result["passwd"] = "$1$" + mAuthenticator["secret"].asString(); result["first"] = mIdentifier["first_name"]; result["last"] = mIdentifier["last_name"]; - + username = result["first"].asString() + " " + result["last"].asString(); } else if (mIdentifier["type"].asString() == "account") { result["username"] = mIdentifier["account_name"]; result["passwd"] = mAuthenticator["secret"]; - + username = result["username"].asString(); } } catch (...) { - LOG_UNHANDLED_EXCEPTION(); + // nat 2016-08-18: not clear what exceptions the above COULD throw?! + LOG_UNHANDLED_EXCEPTION(STRINGIZE("for '" << username << "'")); // we could have corrupt data, so simply return a null login param if so LL_WARNS("AppInit") << "Invalid credential" << LL_ENDL; } diff --git a/indra/newview/llsechandler_basic.cpp b/indra/newview/llsechandler_basic.cpp index 54396cb9a4..d6fb801cc0 100644 --- a/indra/newview/llsechandler_basic.cpp +++ b/indra/newview/llsechandler_basic.cpp @@ -36,9 +36,9 @@ #include "lldir.h" #include "llviewercontrol.h" #include "llexception.h" +#include "stringize.h" #include #include -#include #include #include #include @@ -619,8 +619,7 @@ void LLBasicCertificateStore::load_from_file(const std::string& filename) } catch (...) { - LL_WARNS("SECAPI") << "Failure creating certificate from the certificate store file: " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION("creating certificate from the certificate store file"); } X509_free(cert_x509); cert_x509 = NULL; @@ -1367,8 +1366,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error writing Protected Data Store): " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION("LLProtectedDataException(Error writing Protected Data Store)"); // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future // it may be, however. @@ -1379,29 +1377,28 @@ void LLSecAPIBasicHandler::_writeProtectedData() //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } - try - { - // move the temporary file to the specified file location. - if((( (LLFile::isfile(mProtectedDataFilename) != 0) - && (LLFile::remove(mProtectedDataFilename) != 0))) - || (LLFile::rename(tmp_filename, mProtectedDataFilename))) - { - LL_WARNS() << "LLProtectedDataException(Could not overwrite protected data store)" << LL_ENDL; - LLFile::remove(tmp_filename); + try + { + // move the temporary file to the specified file location. + if((( (LLFile::isfile(mProtectedDataFilename) != 0) + && (LLFile::remove(mProtectedDataFilename) != 0))) + || (LLFile::rename(tmp_filename, mProtectedDataFilename))) + { + LL_WARNS() << "LLProtectedDataException(Could not overwrite protected data store)" << LL_ENDL; + LLFile::remove(tmp_filename); - // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() - // Decided throwing an exception here was overkill until we figure out why this happens - //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); - } + // EXP-1825 crash in LLSecAPIBasicHandler::_writeProtectedData() + // Decided throwing an exception here was overkill until we figure out why this happens + //LLTHROW(LLProtectedDataException("Could not overwrite protected data store")); + } } catch (...) { - LL_WARNS() << "LLProtectedDataException(Error renaming '" << tmp_filename - << "' to '" << mProtectedDataFilename << "'): " - << boost::current_exception_diagnostic_information() << LL_ENDL; + LOG_UNHANDLED_EXCEPTION(STRINGIZE("renaming '" << tmp_filename << "' to '" + << mProtectedDataFilename << "'")); // it's good practice to clean up any secure information on error // (even though this file isn't really secure. Perhaps in the future - // it may be, however. + // it may be, however). LLFile::remove(tmp_filename); //crash in LLSecAPIBasicHandler::_writeProtectedData() @@ -1409,7 +1406,7 @@ void LLSecAPIBasicHandler::_writeProtectedData() //LLTHROW(LLProtectedDataException("Error writing Protected Data Store")); } } - + // instantiate a certificate from a pem string LLPointer LLSecAPIBasicHandler::getCertificate(const std::string& pem_cert) { -- cgit v1.2.3 From ceb9d3f7f11ee659c73cda3fb52231bb8a44476b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Aug 2016 15:46:26 -0400 Subject: MAINT-5011: Fix misleading indentation in WINMAIN(). --- indra/newview/llappviewerwin32.cpp | 46 +++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 23 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index a7f248ab5a..af0cd27fd5 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -328,33 +328,33 @@ int APIENTRY WINMAIN(HINSTANCE hInstance, // app cleanup if there was a problem. // #if WINDOWS_CRT_MEM_CHECKS - LL_INFOS() << "CRT Checking memory:" << LL_ENDL; - if (!_CrtCheckMemory()) - { - LL_WARNS() << "_CrtCheckMemory() failed at prior to cleanup!" << LL_ENDL; - } - else - { - LL_INFOS() << " No corruption detected." << LL_ENDL; - } + LL_INFOS() << "CRT Checking memory:" << LL_ENDL; + if (!_CrtCheckMemory()) + { + LL_WARNS() << "_CrtCheckMemory() failed at prior to cleanup!" << LL_ENDL; + } + else + { + LL_INFOS() << " No corruption detected." << LL_ENDL; + } #endif - - gGLActive = TRUE; - viewer_app_ptr->cleanup(); - + gGLActive = TRUE; + + viewer_app_ptr->cleanup(); + #if WINDOWS_CRT_MEM_CHECKS - LL_INFOS() << "CRT Checking memory:" << LL_ENDL; - if (!_CrtCheckMemory()) - { - LL_WARNS() << "_CrtCheckMemory() failed after cleanup!" << LL_ENDL; - } - else - { - LL_INFOS() << " No corruption detected." << LL_ENDL; - } + LL_INFOS() << "CRT Checking memory:" << LL_ENDL; + if (!_CrtCheckMemory()) + { + LL_WARNS() << "_CrtCheckMemory() failed after cleanup!" << LL_ENDL; + } + else + { + LL_INFOS() << " No corruption detected." << LL_ENDL; + } #endif - + } delete viewer_app_ptr; viewer_app_ptr = NULL; -- cgit v1.2.3 From 4c41b1d8ac041834b37fbd956f646749ac4c0325 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Aug 2016 15:56:47 -0400 Subject: MAINT-5011: Add top-level exception handlers in LLAppViewer::frame(). --- indra/newview/llappviewer.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 766c807169..0c758d73a1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1537,10 +1537,14 @@ bool LLAppViewer::frame() resumeMainloopTimeout(); pingMainloopTimeout("Main:End"); - } + } + } + catch (const LLContinueError&) + { + LOG_UNHANDLED_EXCEPTION(""); } catch(std::bad_alloc) - { + { LLMemory::logMemoryInfo(TRUE) ; //stop memory leaking simulation @@ -1548,7 +1552,7 @@ bool LLAppViewer::frame() LLFloaterReg::findTypedInstance("mem_leaking"); if(mem_leak_instance) { - mem_leak_instance->stop() ; + mem_leak_instance->stop() ; LL_WARNS() << "Bad memory allocation in LLAppViewer::frame()!" << LL_ENDL ; } else @@ -1559,6 +1563,10 @@ bool LLAppViewer::frame() LL_ERRS() << "Bad memory allocation in LLAppViewer::frame()!" << LL_ENDL ; } } + catch (...) + { + CRASH_ON_UNHANDLED_EXCEPTION(""); + } if (LLApp::isExiting()) { @@ -1572,7 +1580,7 @@ bool LLAppViewer::frame() catch(std::bad_alloc) { LL_WARNS() << "Bad memory allocation when saveFinalSnapshot() is called!" << LL_ENDL ; - + //stop memory leaking simulation LLFloaterMemLeak* mem_leak_instance = LLFloaterReg::findTypedInstance("mem_leaking"); @@ -1581,12 +1589,16 @@ bool LLAppViewer::frame() mem_leak_instance->stop() ; } } + catch (...) + { + CRASH_ON_UNHANDLED_EXCEPTION("saveFinalSnapshot()"); + } } - + delete gServicePump; - + destroyMainloopTimeout(); - + LL_INFOS() << "Exiting main_loop" << LL_ENDL; } -- cgit v1.2.3