Stack Overflow in Delphi

Question

I am posting my Stack Overflow problem on StackOverflow.com. Irony at its best!

Anyways. I am calling this procedure on my SkypeReply event handler, which gets fired a lot:

  Procedure OnCategoryRename;
  Var
    CategoryID : Integer;
    sCtgName : String;
  Begin
    if (AnsiContainsStr(pCommand.Reply,'GROUP')) and (AnsiContainsStr(pCommand.Reply,'DISPLAYNAME')) then
      begin
         sCtgName := pCommand.Reply;
         Delete(sCtgName,1,Pos('GROUP',sCtgName)+5);
         CategoryID := StrToInt(Trim(LeftStr(sCtgName,Pos(' ',sCtgName))));
         sCtgName := GetCategoryByID(CategoryID).DisplayName; // Removing THIS line does not produce a Stack Overflow!
         ShowMessage(sCtgName); 
      end;

The idea of this is to loop thru my list of Skype Groups, to see what group has been renamed. AFAIK thats of no importance, as my S.O has been traced to appear here

Function GetCategoryByID(ID : Integer):IGroup;
Var
  I : Integer;
  Category : IGroup;
Begin
  // Make the default result nil
  Result := nil;

  // Loop thru the CUSTOM CATEGORIES of the ONLY SKYPE CONTROL used in this project
  // (which 100% positive IS attached ;) )
  for I := 1 to frmMain.Skype.CustomGroups.Count do
    Begin
      // The Category Variable
      Category := frmMain.Skype.CustomGroups.Item[I];
      // If the current category ID returned by the loop matches the passed ID
      if Category.Id = ID then
        begin
          // Return the Category as Result (IGroup)
          Result := Category;
          // Exit the function.
          Exit;
        end;
    End;
End;

When I set a breakpoint at Result := Category; and Single Step thru, those 2 lines get executed over and over, right after each other!

And when I comment out the sCtgName := GetCategoryByID(CategoryID).DisplayName; in the first code snippet, there is no Overflow, the message gets shown that one time it is supposed to. However, the GetCategoryByID is a function I wrote, and I wrote one similar, too, which works just fine (GetCategoryByName), so I don't get why it decided to repeat the

// Return the Category as Result (IGroup)
Result := Category;
// Exit the function.
Exit;

over and over again.

EDIT: Here is how you can reproduce it: https://gist.github.com/813389

EDIT: Here is my CallStack, as requested:

Edit2: More info:

And if you want to debug this yourself use F7 to step in rather than F8 step over — David Heffernan
– David Heffernan, Commented Feb 5, 2011 at 20:42
I can't see a SO in the code you show. What are you not telling us? — David Heffernan
– David Heffernan, Commented Feb 5, 2011 at 21:07
You can get a stack overflow if you are doing something recursively with no end. OnCategoryRename sounds like an event to me. If there are something in GetCategoryByID that triggers OnCategoryRename you will get a recursion that never ends. You can see if this is happening by looking at the call-stack when stepping through your code. — Mikael Eriksson
– Mikael Eriksson, Commented Feb 5, 2011 at 21:16
What Delphi version? Did you exit Delphi, delete all .DCU files then start Delphi and try your project again? Which Exit are you using (I had a client that had procedure Exit; in a truckload of units). — Jeroen Wiert Pluimers
– Jeroen Wiert Pluimers, Commented Feb 5, 2011 at 22:39
@Jeff: What type is frmMain.Skype.CustomGroups.Item[I]? Can calling it cause the SkypeReply handler to fire? Or can casting it to Category 's type cause SkypeReply to fire? I can see that you are checking whether pCommand.Reply contains 'GROUP', but I don't see the latter being removed from the former (though maybe it's done in the handler's code, if it's needed anyway). If for some reason the handler gets fired before the proc is done (or because the proc causes the handler to fire), is it possible that by that time pCommand.Reply still contains 'GROUP' in it? — Andriy M
– Andriy M, Commented Feb 6, 2011 at 13:18

Thorsten Engler · Accepted Answer · 2011-02-06 02:18:11Z

5

Make sure to compile your project with "optimization" off, "stack frames" on and "use debug .dcu" on to get the most detailed callstack possible. Then post the callstack you get when you hit the stack overflow here (if you have trouble identifying the nature of the problem from it).

answered Feb 6, 2011 at 2:18

Thorsten Engler

2,39112 silver badges13 bronze badges

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

7 Comments

Cosmin Prund Over a year ago

+1, stack overflows are easily identified using the Call Stack.

Jeff Over a year ago

I cant seem to copy the call stack? How do I do that?

Jeff Over a year ago

@David - Yeah, I figured. Just thought there was some particular way of doing it.

Lieven Keersmaekers Over a year ago

... or select all (stack window focused) (CTRL-A) followed by copy (CTRL-C)

Thorsten Engler Over a year ago

The callstack you posted seems to be from when you put a breakpoint on that line and showing the first time it stopped there. If you want to see the recursion that's causing the problem you obviously will need to look at a callstack that actually contains the recursion (in other words, the callstack you get AFTER the stack overflow happened).

|

LeGEC · Accepted Answer · 2011-02-07 12:53:44Z

3

What doesn't show up in your question : the "OnCategoryRename" function you posted up here is a subfunction called from a "TForm.Skype1Reply" callback.

To see this, I had to click on your github link - yet I think it is an important point of your problem.

My guess :

Your "GetCategoryById" function actually sends a query, which triggers "Skype1Reply".
If the groupname has changed, "Skype1Reply" calls "OnCategoryRename".
"OnCategoryRename" calls "GetCategoryById"
"GetCategoryById" triggers "Skype1Reply"
Somehow, the test saying "if groupname has changed" is still true, so "Skype1Reply" calls "OnCategoryRename"
"OnCategoryRename" calls "GetCategoryById"
rinse, repeat

I think a quick and dirty fix would be to change

sCtgName := GetCategoryByID(CategoryID).DisplayName; // Removing THIS line does not produce a Stack Overflow!

with

sCtgName := //find another way to get the new name, which you can probably get from your ICommand object
            pCommand.Reply.ReadDataFromReplyAndGetNewDisplayName;

In the future, I suggest you post your complete code sample for this kind of question.

edited Feb 7, 2011 at 12:53

answered Feb 7, 2011 at 9:33

LeGEC

53.4k5 gold badges69 silver badges127 bronze badges

2 Comments

Jeff Over a year ago

I believe I did mention that, but you are right. I already had a fix before your post however, but since this is the correct answer, thats what it will be marked as. Thanks!

Jeff Over a year ago

Also, the GetCategoryByID will also fire the SkypeReply event, but I used an AnsiContainsStr to check if the first letter of the reply is a #, and if it is, its a recursive call (?). I dont know why it does that however. But it works now :)

Warren P · Accepted Answer · 2011-02-05 21:30:15Z

3

Stack overflows could be caused by endless recursion.

You have to be very careful when you write code that has event handlers in it. One thing you can do to help you debug this is, as David says, step INTO rather than over such calls. F7 steps into a call.

Another thing you can do is put a breakpoint at the top of the function GetCategoryById. Now look at your Call Stack. Do you see the repeated name in the stack? This should make it very clear.

answered Feb 5, 2011 at 21:30

Warren P

69.8k42 gold badges194 silver badges323 bronze badges

Collectives™ on Stack Overflow

Stack Overflow in Delphi

3 Answers 3

7 Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

7 Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related