Stack Overflow in Delphi
Asked Answered
C

3

6

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: CallStack

Edit2: More info: More Info

Caldwell answered 5/2, 2011 at 20:38 Comment(46)
And if you want to debug this yourself use F7 to step in rather than F8 step overAdrell
I can't see a SO in the code you show. What are you not telling us?Adrell
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.Artima
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).Greenhouse
@David - The thing I am not telling you, is that OnCategoryRename is my own procedure, and that is getting called inside an event called SkypeReply, which gets called many times. BUT, as you see the If (Ansi........) then-checks prevent all the code from being executed a billion times. Now, what is supposed to happen, is that the ShowMessage will return the Category Displayname, which is grabbed thru the GetCategoryByID (Which returns the object that contains the Displayname property).Caldwell
@David Now, as I said, when I comment out the GetCategoryByID line, the message gets shown once, containing some of the event text (because the beginning of it was cut off using Delete). I am not calling OnCategoryRename inside the GetCategoryByID. @Jeroen I use Delphi 2010. Will try to delete my DCU's. The "Exit" I use is AFAIK the one that is always used to exit a function. :)Caldwell
@Caldwell There's no reason to suspect that deleting DCUs will help - that's just paranoia. Just set a breakpoint on Result := Category and when it reaches there press F7 and see where you go. Also make use of the Call Stack. Do you know what the call stack is?Adrell
Does the error go away if you remove the ShowMessage in OnCategoryRename?Artima
@David - Well, the reason I had to believe it, would be that doing so fixed another error I had yesterday. :P @Mikael - It does not even REACH the showmessage, when the sCtgName := GetCategoryByID ... is present.Caldwell
@David - I am not completely sure of what the Call Stack is, no.Caldwell
@Caldwell Well, find out what the call stack is and use it. It will lead you to the answer. The most voted up answer tells you to do just that.Adrell
@David - How can I copy the callstack to the clipboard?Caldwell
@Caldwell That's no good. What you need is a stack trace whilst the recursive function calls are in progress. Try looking at the stack trace as you step through the Result := Category recursion.Adrell
@David - Edited. Thats what I got.Caldwell
@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?Tyrelltyrian
@Andriy - CustomGroups.Item is a IGroup type. To be honest I am not sure if it will call that event, I will check it ASAP! The latter is not "removed", but the LeftStr gives me the ID that I need. I also tried Nulling the Reply variable, which did not help either.Caldwell
@Caldwell If the call stack is going to help, you need to descend into the infinite recursion. Suppose that GetCategoryByID indirectly called OnCategoryRename which then called GetCategoryByID which then called OnCategoryRename which then called GetCategoryByID which then called OnCategoryRename etc. Try setting a break point on the entry to GetCategoryByID. Each time you hit it press F9. At some point you'll see GetCategoryByID appearing multiple times in the call stack. At that point you will have the information needed to track down the error.Adrell
@David - You mean set a Breakpoint at sCtgName := GetCategoryByID...?Caldwell
@Caldwell No, somewhere inside GetCategoryByID. You need to know where the repeated calls to it are coming from. Do you actually know what a stack overflow means? In this case it happens because you are (indirectly) calling GetCategoryByID from inside GetCategoryByID.Adrell
@David - Yes, I know what it is. I placed a breakpoint at the Result := Category, where it keeps repeating, but the call stack does not "grow", if thats what you mean? - In fact, the info on there is the same.Caldwell
@Caldwell That doesn't make any sense. You get stack overflows when you have unterminated recursion or when you allocate enormous amounts of data on the stack.Adrell
@David - Will take another Screenshot.Caldwell
@David - New screenie posted, where you can see my Breakpoint, Events, and CallStack. I have stepped thru the Result := Category; many times, the callstack does not change.Caldwell
@Jeff: post a minimal reproducible sample somewhere (like on gist.github.com, or one of the alternatives at stackoverflow.com/questions/687213) so it is easier to help you.Greenhouse
@Caldwell It looks like your SO is somewhere else!Adrell
@Jeroen @David - gist.github.com/813389 - A reproduction of my error.Caldwell
@Jeroen @David - Dont hope it is too much to ask to import the Skype4COM library. :)Caldwell
@Caldwell too much to ask for me I'm afraid!Adrell
@David Yeah, I kinda guessed that. :) If you have an idea of where exactly, and why the S.O is happening, do tell :)Caldwell
@Caldwell what version of Delphi do you use? Does the problem occur when you run without the debugger?Adrell
@David - Delphi 2010! :) Yes, it occurs when I am not debugging, too.Caldwell
@Jeff, is your code called by SKYPE when an event happens? It sure looks that way by the look of the call stack! Maybe the calling thread was created with a very small stack, and it's failing because you're doing something that exceeds the stack space allocated. Try posting a message to your main window and doing all the processing from over there, not from the actual callback.Kellner
@Cosmin - Yes, when an event happens in Skype, it gets sent to my app. I am sure sure what you mean with "Try posting a message to your main window and doing all the processing from over there, not from the actual callback". Also, I found out that when you as much as READ from an IGroup Object, the event gets called again, but I dont know why the Rename messages are being sent again (from Skype to my app).Caldwell
I get a "Not attached" when trying to run your project (with the False parameter in Skype1.Attach changed to True). If running the example requires me (or others) to create a skype account, you're dismissing a lot of volunteers.Altagraciaaltaic
@Lieven - Yes, sorry about that. You need an account in order to use the API. Once you are logged in, press the Allow button for your app to use the API. I dont expect you to create an account just to help me, was hoping that someone among us would have Skype + account already. :)Caldwell
reading from igroup fires an event which reads from igroup?Adrell
@Jeff, you post a message to your main form using PostMessage. I really suspect it's a problem with the stack space: if your message handler would truly get reentered, you'd probably see it in the call stack. You can test the stack space issue by calling a simple method that uses a lot of stack space but does nothing else. Example: A method that allocates an large array as a local variable. Easy to test and worth ruling out if it's not this.Kellner
@Jeff, if you could setup a dummy account to make the example code work out of the box, I will run it again. I don't know anything about Skype besides what it is used for. Setting up a skype account by myself is going to be to much.Altagraciaaltaic
@Caldwell ping me at skype (jpluimers)Greenhouse
@David, @Lieven, @Cosmin, @Jeroen, somehow, weird enough, the same messages are being posted back as Reply to my app, however it has a # char in front, so I wrote some logic to abort if the first char is #. Apparently it works, but not sure if its the best thing to do. Apparently, that recursion happend when I "touch" (read, refference, anything) an IGroup object. I dont know why the same event messages would be repeated however. will report back after some testing.Caldwell
So apparently, it was @Andriy who had the answer.Caldwell
@Jeff, thank you for letting us know what solves it for you.Altagraciaaltaic
@Jeff: Thank you for letting me be convinced once again that sometimes a gut feeling is not to be underestimated.Tyrelltyrian
@Andriy - The least I could do was let you know that it was the problem :)Caldwell
@Andriy: you should make that an answer to @Caldwell can accept it.Greenhouse
@Jeroen: That seems right, but feels odd. I see that it gets lost in the comments, but so much time has passed since, it would look funny now that the real cause is known to be established. I think Jeff could include the info about the cause of the problem into his report he promised us.Tyrelltyrian
F
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.

Fuzee answered 7/2, 2011 at 9:33 Comment(2)
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!Caldwell
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 :)Caldwell
E
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).

Eberhard answered 6/2, 2011 at 2:18 Comment(7)
+1, stack overflows are easily identified using the Call Stack.Kellner
I cant seem to copy the call stack? How do I do that?Caldwell
@David - Yeah, I figured. Just thought there was some particular way of doing it.Caldwell
... or select all (stack window focused) (CTRL-A) followed by copy (CTRL-C)Altagraciaaltaic
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).Eberhard
@Thorsten - I have posted 2 screenies, the small one is after the SO, the second one (big one) shows where the recursion seems to be happening.Caldwell
@Jeff, That callstack does not show any recursion taking place. The only other reason I could see for an stack overflow is if the stack is simply very small (standard for delphi applications is 1MB, but looking at the callstack, the thread you are running on seems to originate in the skype DLL, so I can't say how big the stack is, it might just be 64kb. Can you please show us the threads list to see if there are multiple threads active? Also, assuming that code is NOT being executed in the application main thread, you shouldn't be accessing any controls in it at all!Eberhard
F
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.

Franky answered 5/2, 2011 at 21:30 Comment(0)
F
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.

Fuzee answered 7/2, 2011 at 9:33 Comment(2)
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!Caldwell
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 :)Caldwell

© 2022 - 2024 — McMap. All rights reserved.