StackTrace implementation?

Hi,
I wrote a StackTrace class to use for debugging that would show me a full listing of which functions are called and in what order...
It works (mostly), but I'm wondering if anyone has any better ideas of how to implement it?
Since it should be multi-thread safe, I use CriticalSections around the logging areas; and since I don't want to keep opening & closing the output file, I open it once when I initialize the class and close it when I uninitialize it. It's designed to be Initialized when the program starts, then Uninitialized at the very end of your program. Here's the code:

Code:

---

// StackTrace.h - Interface for the StackTrace class.
///////////////////////////////////////////////////////////////////////////////
#ifndef STACKTRACE_H_INCLUDED_APR_1_2008
#define STACKTRACE_H_INCLUDED_APR_1_2008

#include <windows.h>
#include <fstream>
#include <string>


class StackTrace
{
public:
        StackTrace( const std::string&  func );

        ~StackTrace();

        const std::string& FunctionName() const;

        static bool Initialize( const std::string&  traceFile );
        static bool Uninitialize();

private:
        const std::string                m_Func;

        static std::ofstream        m_TraceFile;
        static CRITICAL_SECTION        m_CS;
        static unsigned int                m_Level;

        StackTrace( const StackTrace& );
        StackTrace& operator=( const StackTrace& );
};


#endif        // STACKTRACE_H_INCLUDED_APR_1_2008

---

Code:

---

// StackTrace.cpp - Implementation for the StackTrace class.
///////////////////////////////////////////////////////////////////////////////
#include <windows.h>
#include "StackTrace.h"


// Define static member variables.
std::ofstream                StackTrace::m_TraceFile;
CRITICAL_SECTION        StackTrace::m_CS;
unsigned int                StackTrace::m_Level = 0;



StackTrace::StackTrace( const std::string&  func )
:        m_Func( func )
{
#ifdef GET_STACK_TRACE
        EnterCriticalSection( &m_CS );

        if ( m_TraceFile.is_open()  == true )
        {
                m_TraceFile << "Entering: [" << m_Level << "] " << m_Func << std::endl;
        }

        ++m_Level;

        LeaveCriticalSection( &m_CS );
#endif        // GET_STACK_TRACE
}


StackTrace::~StackTrace()
{
#ifdef GET_STACK_TRACE
        EnterCriticalSection( &m_CS );

        --m_Level;

        if ( m_TraceFile.is_open()  == true )
        {
                m_TraceFile << "Exiting:  [" << m_Level << "] " << m_Func << std::endl;
        }

        LeaveCriticalSection( &m_CS );
#endif        // GET_STACK_TRACE
}


const std::string&
StackTrace::FunctionName() const
{
        return m_Func;
}


// static
bool
StackTrace::Initialize( const std::string&  traceFile )
{
#ifdef GET_STACK_TRACE
        bool ret = m_TraceFile.is_open();

        if ( ret == false )
        {
                m_TraceFile.open( traceFile.c_str() );

                if ( m_TraceFile.is_open() == true )
                {
                        InitializeCriticalSection( &m_CS );
#        if (_WIN32_WINNT >= 0x0403)
                        SetCriticalSectionSpinCount( &m_CS, 4000 );
#        endif        // _WIN32_WINNT
                        ret = true;
                }
        }

        return ret;
#else
        return true;
#endif        // GET_STACK_TRACE
}


// static
bool
StackTrace::Uninitialize()
{
#ifdef GET_STACK_TRACE
        if ( m_TraceFile.is_open() == true )
        {
                m_TraceFile.close();
                DeleteCriticalSection( &m_CS );
        }
#endif        // GET_STACK_TRACE

        return true;
}

---

The problem I've noticed so far is when I use the class before I call the static Initialize() function, such as in constructors of objects which are global variables...

Here's how you'd typically use this class:

Code:

---

#include "StackTrace.h"

void Func1()
{
    StackTrace trace( "Func1()" );
  ...
}

void Func2()
{
    StackTrace trace( "Func2()" );
  ...
}

int main()
{
    StackTrace::Initialize( "C:\\TraceFile.txt" );
    Func1();
    Func2();
    ...
    StackTrace::Uninitialize();
    return 0;
}

---

Why not make a stacktrace macro, something like this:

Code:

---

#define STACKTRACE()  StackTrace trace(__FUNC__)

---

That way, you get the function name in decorated form without having to type it every time.

[The macro for function name may vary depending on the compiler, so you may have to play around with it. It may also make sense to take another one or two arguments, for line number and filename - the latter at least will help if you have two functions by the same name in different source files (not that I recommend that, of course - but it may help at some point or another)].

--
Mats

Some suggestions.
If you're storing everything in the same log file, then you should also write the thread ID as well. Otherwise seeing
myFunc called
myFunc called
might lead to some counter-intuitive interpretations.

Also consider a 'logging' thread which owns the file, and you send messages to that thread. This may mean you no longer have to deal with critical sections in the logging.

By also routing through a separate logging thread, it becomes much easier (IMO) to implement all sorts of filtering (say ignore a particular thread, or traces below a certain depth).

Quote:

---

Originally Posted by matsp

Why not make a stacktrace macro, something like this:

Code:

---

#define STACKTRACE()  StackTrace trace(__FUNC__)

---

That way, you get the function name in decorated form without having to type it every time.

[The macro for function name may vary depending on the compiler, so you may have to play around with it. It may also make sense to take another one or two arguments, for line number and filename - the latter at least will help if you have two functions by the same name in different source files (not that I recommend that, of course - but it may help at some point or another)].

--
Mats

---

Hey, why didn't I think of that! Thanks, that should work pretty well, that way I could completely remove it in Release builds... :)

Quote:

---

Originally Posted by Salem

Some suggestions.
If you're storing everything in the same log file, then you should also write the thread ID as well. Otherwise seeing
myFunc called
myFunc called
might lead to some counter-intuitive interpretations.

Also consider a 'logging' thread which owns the file, and you send messages to that thread. This may mean you no longer have to deal with critical sections in the logging.

By also routing through a separate logging thread, it becomes much easier (IMO) to implement all sorts of filtering (say ignore a particular thread, or traces below a certain depth).

---

That sounds interesting. I'll have to give it a bit of thought though.

Perhaps you can "register" threads, and then print a name rather than a number. Otherwise, you could call GetCurrentThread to get a handle for the current thread.

I'm also pretty sure that Windows ostream is thread-safe if you build with MT library [and if you don't nearly nothing is thread-safe]. So you could possibly get away with using an InterlockedIncrement() and InterlockedDecrement() to update the level, and avoid using the critical section. Something like this:

Code:

---

        long newlevel = InterlockedDecrement(&m_Level);

        if ( m_TraceFile.is_open()  == true )
        {
                m_TraceFile << "Exiting:  [" << newlevel << "] " << m_Func << std::endl;
        }

---

--
Mats

GetCurrentThreadId for getting the current thread's ID. Handle or ID, it doesn't matter really, though.

Now I've run into another issue -- when the service calls a function in a DLL, the DLL is in a separate address space, so it isn't affected by StackTrace::Initialize() and therefore doesn't trace anything.

I might be able to create some shared memory that other processes could access... but I'm guessing it would be benefitial to write StackTrace info to different files for different processes. So that means I need to call StackTrace::Initialize() as soon as the DLL loads.

I'm not sure which function is the very first one to run for a DLL? I did a search for _DllMainCRTStartup and couldn't find any in source code I'm editing, and the only references to DllMain I saw were for other DLLs, not the one I'm editing.
Any ideas?

Another thing I could do is create a singleton class which calls StackTrace::Initialize() in its constructor & StackTrace::Uninitialize() in its destructor, then create a global variable of that singleton. The only problem I see with that is that, as far as I know, there's no way to control which global variables get created first; so if another global variable gets created which creates a StackTrace object in its constructor before the singleton is created, it will crash when it calls EnterCriticalSection() on an uninitialized CRITICAL_SECTION object.

>> when the service calls a function in a DLL, the DLL is in a separate address space, so it isn't affected by StackTrace::Initialize()
The DLL is in the same address space. It just has it's own copy of StackTrace statics, and thus needs it's own call to Initialize()/Uninitialize().

>> but I'm guessing it would be beneficial to write StackTrace info to different files for different processes.
Nothing wrong with having a unique instance of StackTrace per module (DLL/EXE). If you really wanted one StackTrace instance per process, you could host it in a DLL.

>> and the only references to DllMain I saw were for other DLLs, not the one I'm editing
That DLL may not have an entry point - you can always add a DllMain() if that's the case.

>> Another thing I could do is create a singleton class which calls StackTrace::Initialize() in its constructor
You have to be careful with what you do in a global constructor. Global constructors are called within the context of DllMain(), so you have to follow all the restrictions associated with running inside DllMain(). Creating and using a critical section should be ok - and the CRT should have initialized itself before calling global constructors so using ofstream may be ok (testing will tell for sure).

You also don't have any guarantees that your global object will be construct before other global objects. What I do to get around this type of "catch-22" is to use the "construct on first access" idiom (if you can call it an idiom). A very simple method wold be to add a new static, "m_bInitialized", and have the constructor check the flag and call Initialize() if need be. (The trace file name could be derived from GetModuleFileName()).

If you decide to host StackTrace in a DLL (for a single instance per process), you can still use the same "construct on first access" idiom, you'll just have to figure out how you set the file name.

[edit]
Just noticed the thread's age - not too old though :)
[/edit]

gg