In C++ how can I prevent a function from being called recursively

Question

I have a function which makes use of memory on the heap and it will go badly wrong if it is called before another instance of the same function has completed. How can I prevent this from happening at compile time?

Answer 1

Detecting recursion with any amount determinism of at compile-time is going to be quite difficult. Some static code analysis tools might be able to do it, but even then you can get in to run-time scenarios involving threads that code analyzers won't be able to detect.

You need to detect recursion at run-time. Fundamentally, it's very simple to do this:

bool MyFnSimple()
{
    static bool entered = false;
    if( entered )
    {
        cout << "Re-entered function!" << endl;
        return false;
    }
    entered = true;

    // ...

    entered = false;
    return true;
}

The biggest problem with this, of course, is it is not thread safe. There are a couple of ways to make it thread safe, the simplest being to use a critical section and block the second entry until the first has left. Windows code (no error handling included):

bool MyFnCritSecBlocking()
{
    static HANDLE cs = CreateMutex(0, 0, 0);
    WaitForSingleObject(cs, INFINITE);
    // ... do stuff
    ReleaseMutex(cs);
    return true;
}

If you want the function to return an error when a function has been reentered, you can first test the critsec before grabbing it:

bool MyFnCritSecNonBlocking()
{
    static HANDLE cs = CreateMutex(0, 0, 0);
    DWORD ret = WaitForSingleObject(cs, 0);
    if( WAIT_TIMEOUT == ret )
        return false;   // someone's already in here
    // ... do stuff
    ReleaseMutex(cs);
    return true;
}

There are probably an infinite ways to skin this cat other than the use of static bools and critsecs. One that comes to mind is a combination of testing a local value with one of the Interlocked functions in Windows:

bool MyFnInterlocked()
{
    static LONG volatile entered = 0;
    LONG ret = InterlockedCompareExchange(&entered, 1, 0);
    if( ret == 1 )
        return false;   // someone's already in here
    // ... do stuff
    InterlockedExchange(&entered, 0);
    return false;
}

And, of course, you have to think about exception safety and deadlocks. You don't want a failure in your function to leave it un-enterable by any code. You can wrap any of the constructs above in RAII in order to ensure the release of a lock when an exception or early exit occurs in your function.

UPDATE:

After readong comments I realized I could have included code that illustrates how to implement an RAII solution, since any real code you write is going to use RAII to handle errors. Here is a simple RAII implementation that also illustrates what happens at runtime when things go wrong:

#include <windows.h>
#include <cstdlib>
#include <stdexcept>
#include <iostream>

class CritSecLock
{
public:
    CritSecLock(HANDLE cs) : cs_(cs)
    {
        DWORD ret = WaitForSingleObject(cs_, INFINITE);
        if( ret != WAIT_OBJECT_0 ) 
            throw std::runtime_error("Unable To Acquire Mutex");
        std::cout << "Locked" << std::endl;
    }
    ~CritSecLock()
    {
        std::cout << "Unlocked" << std::endl;
        ReleaseMutex(cs_);
    }
private:
    HANDLE cs_;
};

bool MyFnPrimitiveRAII()
{
    static HANDLE cs = CreateMutex(0, 0, 0);
    try
    {
        CritSecLock lock(cs);
        // ... do stuff
        throw std::runtime_error("kerflewy!");
        return true;
    }
    catch(...)
    {
        // something went wrong 
        // either with the CritSecLock instantiation
        // or with the 'do stuff' code
        std::cout << "ErrorDetected" << std::endl;
        return false;
    }
}

int main()
{
    MyFnPrimitiveRAII();
    return 0;
}