Fixed ICE-5812: refactor capture and printing of exception stack trace

4 files changed, 102 insertions, 108 deletions

diff --git a/cpp/include/IceUtil/Exception.h b/cpp/include/IceUtil/Exception.h
index 5da7f1b00b7..c524ebafdf9 100644
--- a/cpp/include/IceUtil/Exception.h
+++ b/cpp/include/IceUtil/Exception.h
@@ -13,6 +13,7 @@
 #include <IceUtil/Config.h>
 
 #include <exception>
+#include <vector>
 
 namespace IceUtil
 {
@@ -33,14 +34,14 @@ public:
 
     const char* ice_file() const;
     int ice_line() const;
-    const std::string& ice_stackTrace() const;
+    std::string ice_stackTrace() const;
     
 private:
     
     const char* _file;
     int _line;
     static const char* _name;
-    const std::string _stackTrace;
+    const std::vector<void*> _stackFrames;
     mutable ::std::string _str; // Initialized lazily in what().
 };
 
diff --git a/cpp/src/IceUtil/Exception.cpp b/cpp/src/IceUtil/Exception.cpp
index 9a67c79e02a..9543ff939ae 100644
--- a/cpp/src/IceUtil/Exception.cpp
+++ b/cpp/src/IceUtil/Exception.cpp
@@ -32,18 +32,17 @@
 #if defined(__GNUC__) && !defined(__sun) && !defined(__FreeBSD__) && !defined(__MINGW32__)
 #  include <execinfo.h>
 #  include <cxxabi.h>
-#  define ICE_STACK_TRACES
 #  define ICE_GCC_STACK_TRACES
 #endif
 
-#ifdef ICE_WIN32_STACK_TRACES
+#if defined(_WIN32) && !defined(ICE_OS_WINRT)
+#  define ICE_WIN32_STACK_TRACES
 #  if defined(_MSC_VER) && _MSC_VER >= 1700
 #    define DBGHELP_TRANSLATE_TCHAR
 #    include <IceUtil/StringConverter.h>
 #  endif
 #  include <DbgHelp.h>
 #  include <tchar.h>
-#  define ICE_STACK_TRACES
 #endif
 
 using namespace std;
@@ -90,25 +89,58 @@ public:
 
 Init init;
 
-#ifdef ICE_STACK_TRACES
-string
-getStackTrace()
+
+vector<void*>
+getStackFrames()
 {
+    vector<void*> stackFrames;
+
     if(!IceUtilInternal::printStackTraces)
     {
+        return stackFrames;
+    }
+
+#  if defined(ICE_WIN32_STACK_TRACES)
+
+    stackFrames.resize(61);
+    //
+    // 1: skip the first frame (the call to getStackFrames)
+    // 1 + stackSize < 63 on Windows XP according to the documentation for CaptureStackBackTrace
+    //
+    USHORT frameCount = CaptureStackBackTrace(1, static_cast<DWORD>(stackFrames.size()), &stackFrames.front(), 0);
+
+    stackFrames.resize(frameCount);
+
+#  elif defined(ICE_GCC_STACK_TRACES)
+    
+    stackFrames.resize(100);
+    size_t stackDepth = backtrace(&stackFrames.front(), stackFrames.size());
+    stackFrames.resize(stackDepth);
+
+#  endif
+
+    return stackFrames;
+}
+
+
+string
+getStackTrace(const vector<void*>& stackFrames)
+{
+    if(stackFrames.empty())
+    {
         return "";
     }
 
     string stackTrace;
 
-#  ifdef ICE_WIN32_STACK_TRACES
+#  if defined(ICE_WIN32_STACK_TRACES)
+
     //
     // Note: the Sym functions are not thread-safe
     //
     IceUtilInternal::MutexPtrLock<IceUtil::Mutex> lock(globalMutex);
     if(process == 0)
     {
-
         //
         // Compute Search path (best effort)
         // consists of the current working directory, this DLL (or exe) directory and %_NT_SYMBOL_PATH%
@@ -174,102 +206,82 @@ getStackTrace()
     }
     lock.release();
 
-    const int stackSize = 61;
-    void* stack[stackSize];
-
-    //
-    // 1: skip the first frame (the call to getStackTrace)
-    // 1 + stackSize < 63 on XP according to the documentation for CaptureStackBackTrace
-    //
-    USHORT frames = CaptureStackBackTrace(1, stackSize, stack, 0);
-
-    if(frames > 0)
-    {
 #if defined(_MSC_VER) && (_MSC_VER >= 1600)
 #   if defined(DBGHELP_TRANSLATE_TCHAR)
-        static_assert(sizeof(TCHAR) == sizeof(wchar_t), "Bad TCHAR - should be wchar_t");
+    static_assert(sizeof(TCHAR) == sizeof(wchar_t), "Bad TCHAR - should be wchar_t");
 #   else
-        static_assert(sizeof(TCHAR) == sizeof(char), "Bad TCHAR - should be char");
+    static_assert(sizeof(TCHAR) == sizeof(char), "Bad TCHAR - should be char");
 #  endif
 #endif
 
-        char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)];
-
-        SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buffer);
-        symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
-        symbol->MaxNameLen = MAX_SYM_NAME;
-
-        IMAGEHLP_LINE64 line = {};
-        line.SizeOfStruct = sizeof(IMAGEHLP_LINE64);
-        DWORD displacement = 0;
-
-        lock.acquire();
-
-        // TODO: call SymRefreshModuleList here? (not available on XP)
-
+    char buffer[sizeof(SYMBOL_INFO) + MAX_SYM_NAME * sizeof(TCHAR)];
+    
+    SYMBOL_INFO* symbol = reinterpret_cast<SYMBOL_INFO*>(buffer);
+    symbol->SizeOfStruct = sizeof(SYMBOL_INFO);
+    symbol->MaxNameLen = MAX_SYM_NAME;
+    
+    IMAGEHLP_LINE64 line = {};
+    line.SizeOfStruct = sizeof(IMAGEHLP_LINE64);
+    DWORD displacement = 0;
+    
+    lock.acquire();
+    
+    // TODO: call SymRefreshModuleList here? (not available on XP)
+    
 #ifdef DBGHELP_TRANSLATE_TCHAR
-        const IceUtil::StringConverterPtr converter = IceUtil::getProcessStringConverter();
+    const IceUtil::StringConverterPtr converter = IceUtil::getProcessStringConverter();
 #endif
-        for(int i = 0; i < frames; i++)
+    for(size_t i = 0; i < stackFrames.size(); i++)
+    {
+        if(!stackTrace.empty())
+        {
+            stackTrace += "\n";
+        }
+        
+        stringstream s;
+        s << setw(3) << i << " ";
+        
+        DWORD64 address = reinterpret_cast<DWORD64>(stackFrames[i]);
+        
+        BOOL ok = SymFromAddr(process, address, 0, symbol);
+        if(ok)
         {
-            if(!stackTrace.empty())
-            {
-                stackTrace += "\n";
-            }
-
-            stringstream s;
-            s << setw(3) << i << " ";
-
-            DWORD64 address = reinterpret_cast<DWORD64>(stack[i]);
-
-            //
-            // Don't need to use pass a wide string converter in the bellow
-            // calls to wstringToString as the wide strings come from
-            // Windows API.
-            //
-            BOOL ok = SymFromAddr(process, address, 0, symbol);
-            if(ok)
-            {
 #ifdef DBGHELP_TRANSLATE_TCHAR
-                s << IceUtil::wstringToString(symbol->Name, converter);
+            s << IceUtil::wstringToString(symbol->Name, converter);
 #else
-                s << symbol->Name;
+            s << symbol->Name;
 #endif
-                ok = SymGetLineFromAddr64(process, address, &displacement, &line);
-                if(ok)
-                {
-                    s << " at line " << line.LineNumber << " in "
+            ok = SymGetLineFromAddr64(process, address, &displacement, &line);
+            if(ok)
+            {
+                s << " at line " << line.LineNumber << " in "
 #ifdef DBGHELP_TRANSLATE_TCHAR
-                      << IceUtil::wstringToString(line.FileName, converter);
+                  << IceUtil::wstringToString(line.FileName, converter);
 #else
-                      << line.FileName;
+                  << line.FileName;
 #endif
-                }
-            }
-            else
-            {
-                s << hex << "0x" << address;
             }
-            stackTrace += s.str();
         }
-        lock.release();
+        else
+        {
+            s << hex << "0x" << address;
+        }
+        stackTrace += s.str();
     }
+    lock.release();
 
-#   elif defined(ICE_GCC_STACK_TRACES)
-
-    const size_t maxDepth = 100;
-    void *stackAddrs[maxDepth];
+#  elif defined(ICE_GCC_STACK_TRACES)
 
     // With some compilers/toolchains this can fail so we must check that
     // stackStrings is not null.
-    size_t stackDepth = backtrace(stackAddrs, maxDepth);
-    char **stackStrings = backtrace_symbols(stackAddrs, stackDepth);
-    if(stackStrings != NULL)
+    
+    char** stackStrings = backtrace_symbols(&stackFrames.front(), stackFrames.size());
+    if(stackStrings != 0)
     {
         //
         // Start at 1 to skip the top frame (== call to this function)
         //
-        for (size_t i = 1; i < stackDepth; i++)
+        for(size_t i = 1; i < stackFrames.size(); i++)
         {
             string line(stackStrings[i]);
 
@@ -362,38 +374,31 @@ getStackTrace()
             }
 
             stackTrace += s.str();
-
         }
         free(stackStrings);
     }
     else
     {
-        stackTrace += "<stack trace unavailable>";
+        stackTrace = "<stack trace unavailable>";
     }
 
-#   endif
+#  endif
 
     return stackTrace;
 }
-#endif
-
 }
 
 IceUtil::Exception::Exception() :
     _file(0),
-    _line(0)
-#ifdef ICE_STACK_TRACES
-    , _stackTrace(getStackTrace())
-#endif
+    _line(0), 
+    _stackFrames(getStackFrames())
 {
 }
 
 IceUtil::Exception::Exception(const char* file, int line) :
     _file(file),
-    _line(line)
-#ifdef ICE_STACK_TRACES
-    , _stackTrace(getStackTrace())
-#endif
+    _line(line), 
+    _stackFrames(getStackFrames())
 {
 }
 
@@ -465,10 +470,10 @@ IceUtil::Exception::ice_line() const
     return _line;
 }
 
-const string&
+string
 IceUtil::Exception::ice_stackTrace() const
 {
-    return _stackTrace;
+    return getStackTrace(_stackFrames);
 }
 
 ostream&
diff --git a/cpp/src/IceUtil/Makefile.mak b/cpp/src/IceUtil/Makefile.mak
index e9a3b50f799..e606a82bb63 100644
--- a/cpp/src/IceUtil/Makefile.mak
+++ b/cpp/src/IceUtil/Makefile.mak
@@ -45,9 +45,6 @@ CPPFLAGS	= $(CPPFLAGS) -DICE_UTIL_API_EXPORTS -I.. -DWIN32_LEAN_AND_MEAN
 !if "$(GENERATE_PDB)" == "yes"
 PDBFLAGS	= /pdb:$(DLLNAME:.dll=.pdb)
 
-!if "$(WINRT)" != "yes"
-CPPFLAGS 	= $(CPPFLAGS) -DICE_WIN32_STACK_TRACES
-!endif
 !endif
 
 RES_FILE	= IceUtil.res
diff --git a/cpp/test/Freeze/evictor/Server.cpp b/cpp/test/Freeze/evictor/Server.cpp
index 7d869931782..1793f915904 100644
--- a/cpp/test/Freeze/evictor/Server.cpp
+++ b/cpp/test/Freeze/evictor/Server.cpp
@@ -106,16 +106,7 @@ main(int argc, char* argv[])
 
     try
     {
-        Ice::InitializationData initData;
-        initData.properties = Ice::createProperties(argc, argv);
-
-        //
-        // Getting stack traces is expensive and this test creates a lot 
-        // of DeadlockException.
-        //
-        initData.properties->setProperty("Ice.PrintStackTraces", "0");
-
-        communicator = Ice::initialize(argc, argv, initData);
+        communicator = Ice::initialize(argc, argv);
         status = run(argc, argv, communicator, envName);
     }
     catch(const Ice::Exception& ex)