Dealing with infinite recursion

Be sure to check out the follow up post on infinite recursion.

Infinite recursive loops are notoriously difficult bugs to debug. Crashing ASP.NET websites giving little information to go on, and in console and forms applications giving you a handy exception message “An unhandled exception of type ‘System.StackOverflowException’ occurred in Unknown Module.” ‘Unknown Module’, handy.

Once you manage to find the root cause of the infinite recursion, sometimes the fix is easy, it could just be a counter that wasn’t being incremented properly, or some boolean check that wasn’t causing the function to exit. Other times the call stack can be long, with no intermediately obvious solution, especially when in a multi-threaded environment (like ASP.NET.)

For example, on an ASP.NET website I worked on recently, we had a custom configuration reading solution combined with some custom error handling to log errors to the database. A problem arose when then configuration handler failed to read the connection string, the error handling would try to log the error to the database, in doing so it would try to read the connection string from the configuration and fail, and so on and so on. The call stack for this entire operation ended up being quite long, with no particular function being entirely at fault.

Adding a private member variable that would be set when the method was called, and if it was already set would cause the function to exit, wasn’t an option since the class was instantiated each time in the loop, making it static (or VB shared) wouldn’t fix the problem either, since multiple threads where going through the same loop. We came up with a solution under pressure to fix the bug, while not the best solution I’ll show an example of it here for completeness.

Our solution was to keep track of thread IDs in a thread-safe list, if the thread ID was found in the list the function would exit, the final solution looked something like this:

        private static [class]List[/class]<int> _threadList = new [class]List[/class]<int>();

        public void DoWork()
        {
            lock (_threadList)
            {
                int currentId = [class]Thread[/class].CurrentThread.ManagedThreadId;
                if (_threadList.Contains(currentId))
                {
                    _threadList.Remove(currentId);
                    return;
                }
                _threadList.Add(currentId);
            }

            // this eventually leads to infinite recursion
        }

While this worked, it’s not exactly an elegant solution. There’s performance issues related to the locking and the lookup of the thread ID in the collection. This solution was okay, but there’s a better one.

The next solution uses the ThreadStaticAttribute.

From MSDN:

A static field marked with ThreadStaticAttribute is not shared between threads. Each executing thread has a separate instance of the field, and independently sets and gets values for that field. If the field is accessed on a different thread, it will contain a different value.

What that means, in this context, is that I can have a member variable that stays the same so matter how many times I instantiate the class, but changing it won’t change it on other threads. With this information in mind the solution is a lot more simple:

        [[class]ThreadStatic[/class]]
        private static bool _hasBeenCalled;

        public void DoWork()
        {
            if (_hasBeenCalled)
                return;

            _hasBeenCalled = true;

            // this eventually leads to infinite recursion
        }

Now with a simple boolean check, we can be certain that we’re in a loop and exit safely without impacting other threads.

Phil