avoid memory leak in win32 sal::backtrace_get()

Running a presentation with OpenGL transitions with Skia+Vulkan
as the VCL drawing very quickly runs out of memory in dbgutil
builds. The trigger is svl/source/notify/lstner.cxx calling
sal::backtrace_get() quite often. And that function calls
SymInitialize() repeatedly even though its docs say not to do it,
and that is also actually not necessary for CaptureStackBackTrace(),
only for the symbol resolving Sym* functions.
It actually still eventually aborts if called often enough,
but this way it is triggered only by printing the backtrace and
not just getting it.
I have no idea why the problem is triggered only in these rather
specific circumstances, e.g. Skia+raster seems to be fine.
Also avoid the needless copy&paste while I'm at it.

Change-Id: I50f9e0689b9b9b10bf54308db654aed6433085db
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/89626
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

2 files changed, 14 insertions, 63 deletions

diff --git a/sal/osl/unx/backtraceapi.cxx b/sal/osl/unx/backtraceapi.cxx
index ae1670c30b92..93ca94da5ff2 100644
--- a/sal/osl/unx/backtraceapi.cxx
+++ b/sal/osl/unx/backtraceapi.cxx
@@ -36,27 +36,8 @@ struct FreeGuard {
 }
 
 OUString osl::detail::backtraceAsString(sal_uInt32 maxDepth) {
-    assert(maxDepth != 0);
-    auto const maxInt = static_cast<unsigned int>(
-        std::numeric_limits<int>::max());
-    if (maxDepth > maxInt) {
-        maxDepth = static_cast<sal_uInt32>(maxInt);
-    }
-    auto b1 = std::make_unique<void *[]>(maxDepth);
-    int n = backtrace(b1.get(), static_cast<int>(maxDepth));
-    FreeGuard b2(backtrace_symbols(b1.get(), n));
-    b1.reset();
-    if (b2.buffer == nullptr) {
-        return OUString();
-    }
-    OUStringBuffer b3;
-    for (int i = 0; i != n; ++i) {
-        if (i != 0) {
-            b3.append("\n");
-        }
-        b3.append(o3tl::runtimeToOUString(b2.buffer[i]));
-    }
-    return b3.makeStringAndClear();
+    std::unique_ptr<sal::BacktraceState> backtrace = sal::backtrace_get( maxDepth );
+    return sal::backtrace_to_string( backtrace.get());
 }
 
 std::unique_ptr<sal::BacktraceState> sal::backtrace_get(sal_uInt32 maxDepth)
diff --git a/sal/osl/w32/backtrace.cxx b/sal/osl/w32/backtrace.cxx
index 9e31616eaaff..fab3c5043a60 100644
--- a/sal/osl/w32/backtrace.cxx
+++ b/sal/osl/w32/backtrace.cxx
@@ -37,42 +37,8 @@ template<typename T> T clampToULONG(T n) {
 
 OUString osl::detail::backtraceAsString(sal_uInt32 maxDepth)
 {
-    assert(maxDepth != 0);
-    maxDepth = clampToULONG(maxDepth);
-
-    OUStringBuffer aBuf;
-
-    HANDLE hProcess = GetCurrentProcess();
-    SymInitialize( hProcess, nullptr, true );
-
-    std::unique_ptr<void*[]> aStack(new void*[ maxDepth ]);
-    // https://msdn.microsoft.com/en-us/library/windows/desktop/bb204633.aspx
-    // "CaptureStackBackTrace function" claims that you "can capture up to
-    // MAXUSHORT frames", and on Windows Server 2003 and Windows XP it even
-    // "must be less than 63", but assume that a too large input value is
-    // clamped internally, instead of resulting in an error:
-    sal_uInt32 nFrames = CaptureStackBackTrace( 0, static_cast<ULONG>(maxDepth), aStack.get(), nullptr );
-
-    SYMBOL_INFO  * pSymbol;
-    pSymbol = static_cast<SYMBOL_INFO *>(calloc( sizeof( SYMBOL_INFO ) + 1024 * sizeof( char ), 1 ));
-    assert(pSymbol);
-    pSymbol->MaxNameLen = 1024 - 1;
-    pSymbol->SizeOfStruct = sizeof( SYMBOL_INFO );
-
-    for( sal_uInt32 i = 0; i < nFrames; i++ )
-    {
-        SymFromAddr( hProcess, reinterpret_cast<DWORD64>(aStack[ i ]), nullptr, pSymbol );
-        aBuf.append( static_cast<sal_Int32>(nFrames - i - 1) );
-        aBuf.append( ": " );
-        aBuf.appendAscii( pSymbol->Name );
-        aBuf.append( " - 0x" );
-        aBuf.append( static_cast<sal_Int64>(pSymbol->Address), 16 );
-        aBuf.append( "\n" );
-    }
-
-    free( pSymbol );
-
-    return aBuf.makeStringAndClear();
+    std::unique_ptr<sal::BacktraceState> backtrace = sal::backtrace_get( maxDepth );
+    return sal::backtrace_to_string( backtrace.get());
 }
 
 std::unique_ptr<sal::BacktraceState> sal::backtrace_get(sal_uInt32 maxDepth)
@@ -80,9 +46,6 @@ std::unique_ptr<sal::BacktraceState> sal::backtrace_get(sal_uInt32 maxDepth)
     assert(maxDepth != 0);
     maxDepth = clampToULONG(maxDepth);
 
-    HANDLE hProcess = GetCurrentProcess();
-    SymInitialize( hProcess, nullptr, true );
-
     auto pStack = new void *[maxDepth];
     // https://msdn.microsoft.com/en-us/library/windows/desktop/bb204633.aspx
     // "CaptureStackBackTrace function" claims that you "can capture up to
@@ -96,16 +59,23 @@ std::unique_ptr<sal::BacktraceState> sal::backtrace_get(sal_uInt32 maxDepth)
 
 OUString sal::backtrace_to_string(BacktraceState* backtraceState)
 {
-    OUStringBuffer aBuf;
-
+    HANDLE hProcess = GetCurrentProcess();
+    // https://docs.microsoft.com/en-us/windows/win32/api/dbghelp/nf-dbghelp-syminitialize
+    // says to not initialize more than once. This still leaks for some
+    // reason if called often enough.
+    static bool needsInit = true;
+    if( needsInit )
+        SymInitialize( hProcess, nullptr, true );
+    else
+        SymRefreshModuleList( hProcess );
     SYMBOL_INFO  * pSymbol;
     pSymbol = static_cast<SYMBOL_INFO *>(calloc( sizeof( SYMBOL_INFO ) + 1024 * sizeof( char ), 1 ));
     assert(pSymbol);
     pSymbol->MaxNameLen = 1024 - 1;
     pSymbol->SizeOfStruct = sizeof( SYMBOL_INFO );
-    HANDLE hProcess = GetCurrentProcess();
 
     auto nFrames = backtraceState->nDepth;
+    OUStringBuffer aBuf;
     for( int i = 0; i < nFrames; i++ )
     {
         SymFromAddr( hProcess, reinterpret_cast<DWORD64>(backtraceState->buffer[ i ]), nullptr, pSymbol );