Index out of range, how!?

[hanvyj]

I am trying to return the last value of a list:

public int GetControllerState()
        {
            // return the last controllerState in the list
            if (controllerStateList != null)
            {
                if (controllerStateList.Count > 0)
                {
                    return (int)controllerStateList[controllerStateList.Count - 1];
                }
                else
                {
                    return 0;
                }
            }
            else
            {
                return 0;
            }
        }

The above method sometimes throws

"An unhandled exception of type 'System.ArgumentOutOfRangeException' occurred in mscorlib.dll

Additional information: Index was out of range. Must be non-negative and less than the size of the collection."

and breaks on the return statement of the above code, the thing is, when I check the values in the debugger, i can see:

|controllerStateList.Count = 2|
|controllerStateList.Count - 1 = 1|
and |controllerStateList[1] = PERFORMING_RUN|

So, this just shouldnt be happening! Not sure if I am being stupid or not, thanks.

[SaiBalaji]

Try to use break at printf

[hanvyj]

Try to use break at printf

edit: Oh, I see... hope you are having fun

[hanvyj]

It might be worth saying I am accessing this from a different thread? I have declaired the controllerStateList as volatile, Ill try to build a simple example and see if It does it in that

[hanvyj]

post made redundent by below post

[hanvyj]

Well, I'm even more stumped now - I have made a program with two threads, one accessing the list, the other writing to the list (its compilable, you can test it!) And I never get the index error, everything says this error just shouldnt happen! Even the debugger! And that it works sometimes, when performing the same thing...

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace TestingIndexError2
{
    public class Integer //stupid I know
    {
        public int value;
        public Integer(int value)
        {
            this.value = value;
        }
    }

    public class Worker
    {
        private Program program;

        public Worker(Program program)
        {
            this.program = program;
        }

        // This method will be called when the thread is started.
        public void DoWork()
        {
            while (!_shouldStop)
            {
                Console.WriteLine("top: " + program.GetTopOfList());
                Console.WriteLine();
                program.PrintList();
                Console.WriteLine();
            }
            Console.WriteLine("worker thread: terminating.");
        }
        public void RequestStop()
        {
            _shouldStop = true;
        }
        // Volatile is used as hint to the compiler that this data
        // member will be accessed by multiple threads.
        private volatile bool _shouldStop;
    }

    public class Program
    {
        public List<Integer> list;

        static void Main(string[] args)
        {
            Program p = new Program();
            p.Run();
        }

        public void Run()
        {
            // Create the thread object. This does not start the thread.
            Worker workerObject = new Worker(this);
            Thread workerThread = new Thread(workerObject.DoWork);

            // Start the worker thread.
            workerThread.Start();
            Console.WriteLine("main thread: Starting worker thread...");

            // Loop until worker thread activates.
            while (!workerThread.IsAlive) ;

            // Put the main thread to sleep for 1 millisecond to
            // allow the worker thread to do some work:
            Thread.Sleep(1);

            list = new List<Integer>();

            Random random = new Random();
            int i = 0;
            while (i < 10000)
            {
                if (random.Next(0, 2) > 0)
                {
                     // randomly adds an item to top of list
                    list.Add( new Integer(random.Next(0, 10)) );
                }

                if (random.Next(0, 3) > 1)
                {
                    //randomly removes items from top of list
                    RemoveTopOfList();
                }
                Thread.Sleep(10);
                i++;
            }

            // Request that the worker thread stop itself:
            workerObject.RequestStop();

            // Use the Join method to block the current thread 
            // until the object's thread terminates.
            workerThread.Join();
            Console.WriteLine("main thread: Worker thread has terminated.");
        }

        public void PrintList(){
            if (list!=null)
                for (int i = 0 ; i < list.Count ; i++ )
                    Console.Write(list[i].value+ " " );
        }

        public int GetTopOfList()
        {
            // return the last controllerState in the list
            if (list != null)
            {
                if (list.Count > 0)
                {
                    return (list[list.Count - 1]).value;
                }
                else
                {
                    return 0;
                }
            }
            else
            {
                return 0;
            }
        }

        private void RemoveTopOfList()
        {
            //remove the top controllerState
            if (list != null)
            {
                if (list.Count > 0)
                {
                    list.RemoveAt(list.Count - 1);
                }

                if (list.Count == 0)
                {
                    list.Add( new Integer(0) );
                }
            }
        }
    }
}

if you want to try it

[hanvyj]

Success in failing! The following produces the error, i believe that it is the printing that makes this different to the aboce (when commented out it seemed to run ok) the only way I can think that that would effect anything is introduce a delay in the other thread, so i assume i am not making things threadsafe properly...

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;

namespace TestingIndexError2
{
    public class Integer //stupid I know
    {
        public int value;
        public Integer(int value)
        {
            this.value = value;
        }
    }

    public class Worker
    {
        private Program program;

        public Worker(Program program)
        {
            this.program = program;
        }

        // This method will be called when the thread is started.
        public void DoWork()
        {
            int i1 = 0;
            int i2 = 0;

            while (!_shouldStop)
            {
                i1 = program.GetTopOfList();

                if (i1 != i2)
                    Console.WriteLine("top: " + program.GetTopOfList());

                i2 = i1;

                //Console.WriteLine();
                //program.PrintList();
                //Console.WriteLine();
            }
            Console.WriteLine("worker thread: terminating.");
        }
        public void RequestStop()
        {
            _shouldStop = true;
        }
        // Volatile is used as hint to the compiler that this data
        // member will be accessed by multiple threads.
        private volatile bool _shouldStop;
    }

    public class Program
    {
        public List<Integer> list;

        static void Main(string[] args)
        {
            Program p = new Program();
            p.Run();
        }

        public void Run()
        {
            // Create the thread object. This does not start the thread.
            Worker workerObject = new Worker(this);
            Thread workerThread = new Thread(workerObject.DoWork);

            // Start the worker thread.
            workerThread.Start();
            Console.WriteLine("main thread: Starting worker thread...");

            // Loop until worker thread activates.
            while (!workerThread.IsAlive) ;

            // Put the main thread to sleep for 1 millisecond to
            // allow the worker thread to do some work:
            Thread.Sleep(1);

            list = new List<Integer>();

            Random random = new Random();
            int i = 0;
            while (i < 10000)
            {
                if (random.Next(0, 2) > 0)
                {
                    list.Add( new Integer(random.Next(0, 10)) );
                }

                if (random.Next(0, 3) > 1)
                {
                    RemoveTopOfList();
                }
                Thread.Sleep(10);
                i++;
            }

            // Request that the worker thread stop itself:
            workerObject.RequestStop();

            // Use the Join method to block the current thread 
            // until the object's thread terminates.
            workerThread.Join();
            Console.WriteLine("main thread: Worker thread has terminated.");
        }

        public void PrintList(){
            if (list!=null)
                for (int i = 0 ; i < list.Count ; i++ )
                    Console.Write(list[i].value+ " " );
        }

        public int GetTopOfList()
        {
            // return the last controllerState in the list
            if (list != null)
            {
                if (list.Count > 0)
                {
                    if ((list[list.Count - 1]) != null)
                    {
                        return (list[list.Count - 1]).value;
                    }
                    else
                    {
                        return 0;
                    }
                }
                else
                {
                    return 0;
                }
            }
            else
            {
                return 0;
            }
        }

        private void RemoveTopOfList()
        {
            //remove the top controllerState
            if (list != null)
            {
                if (list.Count > 0)
                {
                    list.RemoveAt(list.Count - 1);
                }

                if (list.Count == 0)
                {
                    list.Add( new Integer(0) );
                }
            }
        }
    }
}

[Momerath]

You need to add some form of locking when you are accessing the list. Read this.

[hanvyj]

public int GetTopOfList()
        {
            if (list != null)
            {
                lock (list)
                {
                    if (list.Count > 0)
                    {
                        Integer iI = list[list.Count - 1];
                        if (iI != null)
                        {
                            return iI.value;
                        }
                    }
                }
            }
            return 0;
        }

        private void RemoveTopOfList()
        {
            if (list != null)
            {
                lock (list)
                {
                    if (list.Count > 0)
                    {
                        list.RemoveAt(list.Count - 1);
                    }

                    if (list.Count == 0)
                    {
                        list.Add(new Integer(0));
                    }
                }
            }
        }

Works Thanks!

I have one question though:

Integer iI = list[list.Count - 1];
                        if (iI != null)
                        {
                            return iI.value;
                        }

works, but

if (list[list.Count - 1] != null)
                        {
                            return list[list.Count - 1].value;
                        }

causes a:

A first chance exception of type 'System.NullReferenceException' occurred in TestingIndexError2.exe
An unhandled exception of type 'System.NullReferenceException' occurred in TestingIndexError2.exe

Additional information: Object reference not set to an instance of an object.

But I can't see why..?