diff options
author | Aura Linden <aura@lindenlab.com> | 2014-03-07 14:58:22 -0800 |
---|---|---|
committer | Aura Linden <aura@lindenlab.com> | 2014-03-07 14:58:22 -0800 |
commit | d2bb4dae980a887a30b206875d8f9419901ed66a (patch) | |
tree | dec6222ed82c5e105b69ae9ec64e5f379051734d | |
parent | 8fd270af1cb7ee2cad7c47909b308ef31caf4cd3 (diff) |
Fixes for crash reporter startup race condition, crash reporter CPU use, Secondlife.log filehandle, XP Crash.
-rwxr-xr-x | indra/llcommon/llapp.cpp | 25 | ||||
-rwxr-xr-x | indra/llcommon/llfile.cpp | 31 | ||||
-rwxr-xr-x | indra/llcommon/llfile.h | 2 | ||||
-rwxr-xr-x | indra/llcrashlogger/llcrashlogger.cpp | 53 | ||||
-rwxr-xr-x | indra/llvfs/lldir.cpp | 26 | ||||
-rwxr-xr-x | indra/newview/llappviewer.cpp | 20 | ||||
-rw-r--r-- | indra/newview/llappviewerwin32.cpp | 33 | ||||
-rwxr-xr-x | indra/newview/llfloaterspellchecksettings.cpp | 35 | ||||
-rwxr-xr-x | indra/newview/llfloaterspellchecksettings.h | 1 | ||||
-rw-r--r-- | indra/win_crash_logger/llcrashloggerwindows.cpp | 35 | ||||
-rwxr-xr-x | indra/win_crash_logger/win_crash_logger.cpp | 2 |
11 files changed, 160 insertions, 103 deletions
diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 2c5da5d2a7..5c8fff051f 100755 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -342,12 +342,13 @@ void LLApp::setupErrorHandling() std::wstring wpipe_name; wpipe_name = mCrashReportPipeStr + wstringize(getPid()); - ::Sleep(3000); //HACK hopefully a static wait won't blow up in my face before google fixes their implementation. const std::wstring wdump_path(wstringize(mDumpPath)); - //HACK this for loop is ueless. Breakpad dumbly returns success when the OOP handler isn't initialized. - for (int retries=0;retries<5;++retries) + int retries = 30; + for (; retries > 0; --retries) { + if (mExceptionHandler != 0) delete mExceptionHandler; + mExceptionHandler = new google_breakpad::ExceptionHandler( wdump_path, NULL, //No filter @@ -357,25 +358,20 @@ void LLApp::setupErrorHandling() MiniDumpNormal, //Generate a 'normal' minidump. wpipe_name.c_str(), NULL); //No custom client info. - if (mExceptionHandler) + if (mExceptionHandler->IsOutOfProcess()) { + LL_INFOS("CRASHREPORT") << "Successfully attached to Out of Process exception handler." << LL_ENDL; break; } else { + LL_WARNS("CRASHREPORT") << "Unable to attach to Out of Process exception handler. " << retries << " retries remaining." << LL_ENDL; ::Sleep(100); //Wait a tick and try again. } } - if (!mExceptionHandler) - { - llwarns << "Failed to initialize OOP exception handler. Defaulting to In Process handling" << llendl; - mExceptionHandler = new google_breakpad::ExceptionHandler( - wstringize(mDumpPath), - 0, //dump filename - windows_post_minidump_callback, - 0, - google_breakpad::ExceptionHandler::HANDLER_ALL); - } + + if (retries == 0) LL_WARNS("CRASHREPORT") << "Unable to attach to Out of Process exception handler." << LL_ENDL; + if (mExceptionHandler) { mExceptionHandler->set_handle_debug_exceptions(true); @@ -991,6 +987,7 @@ bool windows_post_minidump_callback(const wchar_t* dump_path, S32 remaining = LLApp::MAX_MINDUMP_PATH_LENGTH; size_t bytesUsed; + LL_INFOS("MINIDUMPCALLBACK") << "Dump file was generated." << LL_ENDL; bytesUsed = wcstombs(path, dump_path, static_cast<size_t>(remaining)); remaining -= bytesUsed; path += bytesUsed; diff --git a/indra/llcommon/llfile.cpp b/indra/llcommon/llfile.cpp index c3a0f0bfe0..761d7f430c 100755 --- a/indra/llcommon/llfile.cpp +++ b/indra/llcommon/llfile.cpp @@ -265,6 +265,37 @@ int LLFile::rename(const std::string& filename, const std::string& newname) return warnif(STRINGIZE("rename to '" << newname << "' from"), filename, rc); } +bool LLFile::copy(const std::string from, const std::string to) +{ + bool copied = false; + LLFILE* in = LLFile::fopen(from, "rb"); /* Flawfinder: ignore */ + if (in) + { + LLFILE* out = LLFile::fopen(to, "wb"); /* Flawfinder: ignore */ + if (out) + { + char buf[16384]; /* Flawfinder: ignore */ + size_t readbytes; + bool write_ok = true; + while(write_ok && (readbytes = fread(buf, 1, 16384, in))) /* Flawfinder: ignore */ + { + if (fwrite(buf, 1, readbytes, out) != readbytes) + { + LL_WARNS("LLFile") << "Short write" << LL_ENDL; + write_ok = false; + } + } + if ( write_ok ) + { + copied = true; + } + fclose(out); + } + fclose(in); + } + return copied; +} + int LLFile::stat(const std::string& filename, llstat* filestatus) { #if LL_WINDOWS diff --git a/indra/llcommon/llfile.h b/indra/llcommon/llfile.h index d59e68367e..f56b22bf9a 100755 --- a/indra/llcommon/llfile.h +++ b/indra/llcommon/llfile.h @@ -75,6 +75,8 @@ public: static int rmdir(const std::string& filename); static int remove(const std::string& filename); static int rename(const std::string& filename,const std::string& newname); + static bool copy(const std::string from, const std::string to); + static int stat(const std::string& filename,llstat* file_status); static bool isdir(const std::string& filename); static bool isfile(const std::string& filename); diff --git a/indra/llcrashlogger/llcrashlogger.cpp b/indra/llcrashlogger/llcrashlogger.cpp index bd34caf241..e66b7cbba4 100755 --- a/indra/llcrashlogger/llcrashlogger.cpp +++ b/indra/llcrashlogger/llcrashlogger.cpp @@ -218,21 +218,10 @@ void LLCrashLogger::gatherFiles() { // Figure out the filename of the second life log LLCurl::setCAFile(gDirUtilp->getCAFile()); - mFileMap["SecondLifeLog"] = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,"SecondLife.log"); + mFileMap["SecondLifeLog"] = gDirUtilp->getExpandedFilename(LL_PATH_DUMP,"SecondLife.log"); mFileMap["SettingsXml"] = gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS,"settings.xml"); } - if(mCrashInPreviousExec) - { - // Restarting after freeze. - // Replace the log file ext with .old, since the - // instance that launched this process has overwritten - // SecondLife.log - std::string log_filename = mFileMap["SecondLifeLog"]; - log_filename.replace(log_filename.size() - 4, 4, ".old"); - mFileMap["SecondLifeLog"] = log_filename; - } - gatherPlatformSpecificFiles(); //Use the debug log to reconstruct the URL to send the crash report to @@ -271,7 +260,7 @@ void LLCrashLogger::gatherFiles() std::ifstream f((*itr).second.c_str()); if(!f.is_open()) { - std::cout << "Can't find file " << (*itr).second << std::endl; + LL_INFOS("CRASHREPORT") << "Can't find file " << (*itr).second << LL_ENDL; continue; } std::stringstream s; @@ -488,6 +477,12 @@ bool LLCrashLogger::sendCrashLogs() else { //mCrashInfo["DebugLog"].erase("MinidumpPath"); + //To preserve logfile on clean shutdown move to regular log dir. + std::string curr_log = (*lock)["dumpdir"].asString() + "SecondLife.log"; + std::string last_log = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,"SecondLife.log"); + + LLFile::remove(last_log); + LLFile::rename(curr_log, last_log); //Before we blow away the directory, perserve log of previous run. mKeyMaster.cleanupProcess((*lock)["dumpdir"].asString()); } @@ -529,11 +524,25 @@ bool LLCrashLogger::init() // Default to the product name "Second Life" (this is overridden by the -name argument) mProductName = "Second Life"; + // Rename current log file to ".old" + std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "crashreport.log.old"); + std::string log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "crashreport.log"); + +#if LL_WINDOWS + LLAPRFile::remove(old_log_file); +#endif + + LLFile::rename(log_file.c_str(), old_log_file.c_str()); + + // Set the log file to crashreport.log + LLError::logToFile(log_file); //NOTE: Until this line, LL_INFOS LL_WARNS, etc are blown to the ether. + // Handle locking - bool locked = mKeyMaster.requestMaster(); //Request maser locking file. wait time is defaulted to 300S + bool locked = mKeyMaster.requestMaster(); //Request master locking file. wait time is defaulted to 300S while (!locked && mKeyMaster.isWaiting()) { + LL_INFOS("CRASHREPORT") << "Waiting for lock." << LL_ENDL; #if LL_WINDOWS Sleep(1000); #else @@ -544,22 +553,9 @@ bool LLCrashLogger::init() if (!locked) { - llwarns << "Unable to get master lock. Another crash reporter may be hung." << llendl; + LL_WARNS("CRASHREPORT") << "Unable to get master lock. Another crash reporter may be hung." << LL_ENDL; return false; } - - // Rename current log file to ".old" - std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "crashreport.log.old"); - std::string log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "crashreport.log"); - -#if LL_WINDOWS - LLAPRFile::remove(old_log_file); -#endif - - LLFile::rename(log_file.c_str(), old_log_file.c_str()); - - // Set the log file to crashreport.log - LLError::logToFile(log_file); mCrashSettings.declareS32("CrashSubmitBehavior", CRASH_BEHAVIOR_ALWAYS_SEND, "Controls behavior when viewer crashes " @@ -587,5 +583,6 @@ bool LLCrashLogger::init() // For cleanup code common to all platforms. void LLCrashLogger::commonCleanup() { + LLError::logToFile(""); //close crashreport.log LLProxy::cleanupClass(); } diff --git a/indra/llvfs/lldir.cpp b/indra/llvfs/lldir.cpp index ccdfd2ab3c..0d65c3f8c3 100755 --- a/indra/llvfs/lldir.cpp +++ b/indra/llvfs/lldir.cpp @@ -187,8 +187,30 @@ S32 LLDir::deleteFilesInDir(const std::string &dirname, const std::string &mask) U32 LLDir::deleteDirAndContents(const std::string& dir_name) { - //Removes the directory and its contents. Returns number of files removed. - return boost::filesystem::remove_all(dir_name); + //Removes the directory and its contents. Returns number of files deleted. + + U32 num_deleted = 0; + + try + { + boost::filesystem::path dir_path(dir_name); + if (boost::filesystem::exists (dir_path)) + { + if (!boost::filesystem::is_empty (dir_path)) + { // Directory has content + num_deleted = boost::filesystem::remove_all (dir_path); + } + else + { // Directory is empty + boost::filesystem::remove (dir_path); + } + } + } + catch (boost::filesystem::filesystem_error &er) + { + llwarns << "Failed to delete " << dir_name << " with error " << er.code().message() << llendl; + } + return num_deleted; } const std::string LLDir::findFile(const std::string &filename, diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 537142ebde..c31c0990c7 100755 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1940,7 +1940,6 @@ bool LLAppViewer::cleanup() gDirUtilp->deleteFilesInDir(gDirUtilp->getExpandedFilename(LL_PATH_CACHE,""), "*.*"); } - removeDumpDir(); writeDebugInfo(); LLLocationHistory::getInstance()->save(); @@ -2102,6 +2101,14 @@ bool LLAppViewer::cleanup() llinfos << "Goodbye!" << llendflush; + //To preserve logfile on clean shutdown move to regular log dir. + std::string curr_log = gDirUtilp->getExpandedFilename(LL_PATH_DUMP,"SecondLife.log"); + std::string last_log = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,"SecondLife.log"); + LLError::logToFile(""); //Close Secondlife.log + LLFile::remove(last_log); + LLFile::copy(curr_log, last_log); + removeDumpDir(); + // return 0; return true; } @@ -2191,7 +2198,7 @@ void LLAppViewer::initLoggingAndGetLastDuration() // Get name of the log file std::string log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log"); - /* + /* * Before touching any log files, compute the duration of the last run * by comparing the ctime of the previous start marker file with the ctime * of the last log file. @@ -2237,6 +2244,8 @@ void LLAppViewer::initLoggingAndGetLastDuration() // Rename current log file to ".old" LLFile::rename(log_file, old_log_file); + log_file = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, + "SecondLife.log"); // Set the log file to SecondLife.log LLError::logToFile(log_file); if (!duration_log_msg.empty()) @@ -3505,7 +3514,7 @@ void LLAppViewer::handleViewerCrash() LL_WARNS("MarkerFile") << "No gDirUtilp with which to create error marker file name" << LL_ENDL; } -#ifdef LL_WINDOWS //SPATTERS Wild guess that filename for Breakpad is not being returned due to sleep cycle in Crash Reporter. +#ifdef LL_WINDOWS Sleep(2000); #endif @@ -3514,12 +3523,9 @@ void LLAppViewer::handleViewerCrash() if(minidump_file && minidump_file[0] != 0) { gDebugInfo["Dynamic"]["MinidumpPath"] = minidump_file; - //SPATTERS another possibility is that when using OOP it must be initiated by a wrapping program so that when the - //viewer crashes, we are from a sibling thread than as a child. Might be able to request minidump at this point - //as a work-around. } #ifdef LL_WINDOWS - else //SPATTERS there is no else here in the older code + else { getFileList(); } diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index c861d0a99f..0e4efa34c7 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -276,7 +276,7 @@ int APIENTRY WINMAIN(HINSTANCE hInstance, viewer_app_ptr->setErrorHandler(LLAppViewer::handleViewerCrash); #if LL_SEND_CRASH_REPORTS - ::SetUnhandledExceptionFilter(catchallCrashHandler); + // ::SetUnhandledExceptionFilter(catchallCrashHandler); #endif // Set a debug info flag to indicate if multiple instances are running. @@ -698,8 +698,35 @@ void LLAppViewerWin32::initCrashReporting(bool reportFreeze) { logdir = logdir.substr(0,end+1); } - std::string arg_str = "\"" + exe_path + "\" -dumpdir \"" + logdir + "\" -procname \"" + appname + "\" -pid " + stringize(LLApp::getPid()); - _spawnl(_P_NOWAIT, exe_path.c_str(), arg_str.c_str(), NULL); + //std::string arg_str = "\"" + exe_path + "\" -dumpdir \"" + logdir + "\" -procname \"" + appname + "\" -pid " + stringize(LLApp::getPid()); + //_spawnl(_P_NOWAIT, exe_path.c_str(), arg_str.c_str(), NULL); + std::string arg_str = "\"" + exe_path + "\" -dumpdir \"" + logdir + "\" -procname \"" + appname + "\" -pid " + stringize(LLApp::getPid()); + + STARTUPINFO startInfo={sizeof(startInfo)}; + PROCESS_INFORMATION processInfo; + + std::wstring exe_wstr; + exe_wstr=wstringize(exe_path); + + std::wstring arg_wstr; + arg_wstr=wstringize(arg_str); + + LL_INFOS("CrashReport") << "Creating crash reporter process " << exe_path << " with params: " << arg_str << LL_ENDL; + if(CreateProcess(exe_wstr.c_str(), + &arg_wstr[0], // Application arguments + 0, + 0, + FALSE, + CREATE_DEFAULT_ERROR_MODE, + 0, + 0, // Working directory + &startInfo, + &processInfo) == FALSE) + // Could not start application -> call 'GetLastError()' + { + LL_WARNS("CrashReport Launch") << "CreateProcess failed " << GetLastError() << LL_ENDL; + return; + } } //virtual diff --git a/indra/newview/llfloaterspellchecksettings.cpp b/indra/newview/llfloaterspellchecksettings.cpp index 5ecdd11918..54c7b4c37d 100755 --- a/indra/newview/llfloaterspellchecksettings.cpp +++ b/indra/newview/llfloaterspellchecksettings.cpp @@ -307,12 +307,12 @@ void LLFloaterSpellCheckerImport::onBtnOK() else { std::string settings_dic = LLSpellChecker::getDictionaryUserPath() + mDictionaryBasename + ".dic"; - if ( copyFile( dict_dic, settings_dic ) ) + if ( LLFile::copy( dict_dic, settings_dic ) ) { if (gDirUtilp->fileExists(dict_aff)) { std::string settings_aff = LLSpellChecker::getDictionaryUserPath() + mDictionaryBasename + ".aff"; - if (copyFile( dict_aff, settings_aff )) + if ( LLFile::copy( dict_aff, settings_aff )) { imported = true; } @@ -385,37 +385,6 @@ void LLFloaterSpellCheckerImport::onBtnOK() closeFloater(false); } -bool LLFloaterSpellCheckerImport::copyFile(const std::string from, const std::string to) -{ - bool copied = false; - LLFILE* in = LLFile::fopen(from, "rb"); /* Flawfinder: ignore */ - if (in) - { - LLFILE* out = LLFile::fopen(to, "wb"); /* Flawfinder: ignore */ - if (out) - { - char buf[16384]; /* Flawfinder: ignore */ - size_t readbytes; - bool write_ok = true; - while(write_ok && (readbytes = fread(buf, 1, 16384, in))) /* Flawfinder: ignore */ - { - if (fwrite(buf, 1, readbytes, out) != readbytes) - { - LL_WARNS("SpellCheck") << "Short write" << LL_ENDL; - write_ok = false; - } - } - if ( write_ok ) - { - copied = true; - } - fclose(out); - } - } - fclose(in); - return copied; -} - std::string LLFloaterSpellCheckerImport::parseXcuFile(const std::string& file_path) const { LLXMLNodePtr xml_root; diff --git a/indra/newview/llfloaterspellchecksettings.h b/indra/newview/llfloaterspellchecksettings.h index eded3a9133..de59d83f24 100755 --- a/indra/newview/llfloaterspellchecksettings.h +++ b/indra/newview/llfloaterspellchecksettings.h @@ -58,7 +58,6 @@ protected: void onBtnBrowse(); void onBtnCancel(); void onBtnOK(); - bool copyFile(const std::string from, const std::string to); std::string parseXcuFile(const std::string& file_path) const; std::string mDictionaryDir; diff --git a/indra/win_crash_logger/llcrashloggerwindows.cpp b/indra/win_crash_logger/llcrashloggerwindows.cpp index b72f5be853..e3356f90ba 100644 --- a/indra/win_crash_logger/llcrashloggerwindows.cpp +++ b/indra/win_crash_logger/llcrashloggerwindows.cpp @@ -260,8 +260,7 @@ LLCrashLoggerWindows::~LLCrashLoggerWindows(void) bool LLCrashLoggerWindows::getMessageWithTimeout(MSG *msg, UINT to) { bool res; - const int timerID=37; - SetTimer(NULL, timerID, to, NULL); + UINT_PTR timerID = SetTimer(NULL, NULL, to, NULL); res = GetMessage(msg, NULL, 0, 0); KillTimer(NULL, timerID); if (!res) @@ -273,7 +272,10 @@ bool LLCrashLoggerWindows::getMessageWithTimeout(MSG *msg, UINT to) int LLCrashLoggerWindows::processingLoop() { const int millisecs=1000; - static int first_connect = 1; + int retries = 0; + const int max_retries = 60; + + LL_DEBUGS("CRASHREPORT") << "Entering processing loop for OOP server" << LL_ENDL; LLSD options = getOptionData( LLApp::PRIORITY_COMMAND_LINE ); @@ -290,11 +292,17 @@ int LLCrashLoggerWindows::processingLoop() { DispatchMessage(&msg); } - if (first_connect ) + if ( retries < max_retries ) //Wait up to 1 minute for the viewer to say hello. { - if ( mClientsConnected > 0) + if (mClientsConnected == 0) + { + LL_DEBUGS("CRASHREPORT") << "Waiting for client to connect." << LL_ENDL; + ++retries; + } + else { - first_connect = 0; + LL_INFOS("CRASHREPORT") << "Client has connected!" << LL_ENDL; + retries = max_retries; } } else @@ -311,9 +319,7 @@ int LLCrashLoggerWindows::processingLoop() { } llinfos << "session ending.." << llendl; - - llinfos << "clients connected :" << mClientsConnected << llendl; - + return 0; } @@ -321,16 +327,15 @@ int LLCrashLoggerWindows::processingLoop() { void LLCrashLoggerWindows::OnClientConnected(void* context, const google_breakpad::ClientInfo* client_info) { - llinfos << "client start. pid = " << client_info->pid() << llendl; sInstance->mClientsConnected++; + LL_INFOS("CRASHREPORT") << "Client connected. pid = " << client_info->pid() << " total clients " << sInstance->mClientsConnected << LL_ENDL; } void LLCrashLoggerWindows::OnClientExited(void* context, const google_breakpad::ClientInfo* client_info) { - llinfos << "client end. pid = " << client_info->pid() << llendl; - sInstance->mClientsConnected--; + LL_INFOS("CRASHREPORT") << "Client disconnected. pid = " << client_info->pid() << " total clients " << sInstance->mClientsConnected << LL_ENDL; } @@ -402,19 +407,21 @@ bool LLCrashLoggerWindows::initCrashServer() return false; } + LL_INFOS("CRASHREPORT") << "Initialized OOP server with pipe named " << stringize(wpipe_name) << LL_ENDL; return true; } bool LLCrashLoggerWindows::init(void) { - initCrashServer(); bool ok = LLCrashLogger::init(); if(!ok) return false; + initCrashServer(); + /* mbstowcs( gProductName, mProductName.c_str(), LL_ARRAY_SIZE(gProductName) ); gProductName[ LL_ARRY_SIZE(gProductName) - 1 ] = 0; - swprintf(gProductName, L"Second Life"); + swprintf(gProductName, L"Second Life"); */ llinfos << "Loading dialogs" << llendl; diff --git a/indra/win_crash_logger/win_crash_logger.cpp b/indra/win_crash_logger/win_crash_logger.cpp index 0f125028a3..79b8514725 100755 --- a/indra/win_crash_logger/win_crash_logger.cpp +++ b/indra/win_crash_logger/win_crash_logger.cpp @@ -34,7 +34,7 @@ int APIENTRY WinMain(HINSTANCE hInstance, LPSTR lpCmdLine, int nCmdShow) { - llinfos << "Starting crash reporter." << llendl; + llinfos << "Starting crash reporter with args" << &lpCmdLine << llendl; LLCrashLoggerWindows app; app.setHandle(hInstance); app.parseCommandOptions(__argc, __argv); |