StackOverFlow in recursive function C#

Question

I have this code, but there is an recursive function and i can't optimize it, that's why i always have StackOverFlow exception. Can you help me to optimize it? I use this void to get all info or to try again if some errors are found.

 public void Parse(byte[] data)
    {
        Socket skt = new Socket(SocketType.Dgram, ProtocolType.Udp);
        skt.ReceiveTimeout = 1000;

        skt.Connect(Encryptor.GetNextIP(), port);
        skt.Send(data);
        try
        {
         //a lot of actions to get the opcode
            switch (opCode)
            {
                case 0x01:
                    p = new Packet(opCode);
                    p.WriteArray(Encryptor.ClientData(answer, false));
                    Regex rgx = new Regex("\"(?<Name>.+?):");
                    string part = p.ReadAString();
                    string res;
                    if (part[part.Length - 1] != '}') part += '}';
                    if (rgx.Match(part).Groups["Name"].Value == "")
                    {
                        p = null;
                        Parse(data);
                        break;
                    }
                    else
                    { res = "ID=" + rgx.Match(part).Groups["Name"].Value; }
                    ParseInfo(rgx.Match(part).Groups["Name"].Value);
                    p = null;
                    Encryptor = null;
                    break;
                default: string ans = Encoding.ASCII.GetString(recv.Take(bytes).ToArray());
                    if (ans.Contains("EndOfStream") || ans.Contains("Begin"))
                    {
                        Failed();
                        Encryptor = null;
                        break;
                    }
                    else
                    {
                        p = null;
                        Parse(data);
                    }
                    break;
            }
        }
        catch
        {
            skt.Close();
            skt.Dispose();
            GC.Collect();
            Parse(data);
        }
    }

Thank you in advance.

Simple: don't use recursion. And last place to use it is in a general catch block. The Dispose() belongs in a finally block, the GC.Collect() behind a // — Henk Holterman
– Henk Holterman, Commented Feb 25, 2014 at 20:15
Using meaningful variable names is strongly suggested in C#, and abr dt cnt. — Magus
– Magus, Commented Feb 25, 2014 at 20:15
One thing that worries me is that you are calling the same method with the same data in the catch statement. Surely if you reach an exception this will just keep on going? — Rob Aston
– Rob Aston, Commented Feb 25, 2014 at 20:16
As Rob Aston pointed out and and selkathguy said in a solution, it's generally poor form to try calling the same method in a Catch block... Why would anything be different the next time you call it? Also you probably shouldn't catch and ignore EVERY exception like you are. Step through your code and see why it threw something, this is probably the problem. — sab669
– sab669, Commented Feb 25, 2014 at 20:18

LB2 · Accepted Answer · 2014-02-25 20:16:35Z

1

All recursive functions must have and reach a terminating condition to rollback the stack. You left out how opCode value is derived, but if that one is ALWAYS 0x01 with regex evaluating to true, then function will never reach terminating condition. Same for default case and never seeing begin or EOS.

Another way of looking at it is that with every recursive call, it must take you one step closer to the end.

So examine your code logic to see why you never reach terminating condition.

also, blank catch that assumes memory fault that later starts recursion again is also possibly at fault... if socket returns some generic exception (no connectivity) then you'll recurse forever (until stack is blown)

answered Feb 25, 2014 at 20:16

LB2

4,86023 silver badges37 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Nathan · Accepted Answer · 2014-02-25 20:17:08Z

1

You appear to be calling Parse(data) without ever modifying the contents of data. It makes sense that it would call itself forever, especially considering the call to Parse(data) in your catch block. I don't see any base case in your code that would cause the recursion to stop.

Also, I really don't recommend calling GC.Collect(); Let the runtime handle memory management.

answered Feb 25, 2014 at 20:17

Nathan

2,1033 gold badges21 silver badges33 bronze badges

Comments

selkathguy · Accepted Answer · 2014-02-25 20:30:27Z

1

Your Try/Catch block is catching an exception from your code, then calling the same function that throws the exception before it has been popped from the execution stack. This is causing your StackOverflowException.

You have a Try/Catch block within a function, who's Catch segment calls the function containing it. This is generally very dangerous, and will hide the true exception of the program. For debugging purposes, try commenting out the try/catch and letting your debugger raise the exception to you so that you can get the source site of the error and track down what's going on.

Moving forward, if your program cannot handle a problem with the socket and is absolutely forced to throw an exception, don't try to shove the data back into the function. I recommend letting the socket die, clear your buffers, and resetting the connection. Then you can log the error to a file, or show a message to the user about the failure.

edited Feb 25, 2014 at 20:30

answered Feb 25, 2014 at 20:15

selkathguy

1,1717 silver badges18 bronze badges

2 Comments

Ronan Thibaudau Over a year ago

This should be a comment, not an answer, as it has no relevance to the question being asked (StackOverflowException)

selkathguy Over a year ago

Good catch, I was responding to the subject line and not the actual question (about optimization) in the OP. I assumed from the subject that the issue was the StackOverflowException. Thanks!

Community · Accepted Answer · 2017-05-23 12:28:21Z

0

You cannot recurse forever, if you want to do something forever you need to change your logic from "i want to do this, and then i'll call this again and again" to some form of while(true) do this in a loop.

So change your logic into an infinite loop and instead of calling Parse within it, just don't (you will end up in the same place since your loop will get back in the same place and also have access to data)

Let's go with a minimalistic example instead of re typing all your code :

public void Parse(string data)
{
 var NewData = data + " again";
 Parse("test"); // this will call itself over and over resulting in a stackoverflow exception since the stack has to be preserved for the whole chain of functions
}

Change it to something like:

public void Parse(string data)
{
var NewData = data + " again";
while(true)
{
 NewData = data + " again"; // Will still run forever but within the same function, no stackoverflow
}
}

I suggest you read this thread for more info : What is tail recursion?

edited May 23, 2017 at 12:28

CommunityBot

11 silver badge

answered Feb 25, 2014 at 20:21

Ronan Thibaudau

3,6013 gold badges34 silver badges79 bronze badges

2 Comments

AsTex Over a year ago

You don't understand. When i call ParseInfo process finished, when i call Failed() - too. But when i call again Parse(data) it means that we should retry.

Ronan Thibaudau Over a year ago

It doesn't change anything at all, in either case you want to get out of recursion IF the recursion can be infinite, basically within the while true, add a check, if it failed, do nothing (the loop continues) else, brea out of the loop (and then call whatever you want to indicate things went well). This isn't an example of what you should do functionaly, it's a technical example, rewrite your code to get out of "run this, and if it failed, run this again" and instead write "as long as i failed (while) keep running (code of fail in the loop) then exit loop and continue with success code)

Collectives™ on Stack Overflow

StackOverFlow in recursive function C#

4 Answers 4

Comments

Comments

2 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related