How to debug a Not Responding program element

[Willicious 8]

6 hours ago, Kas Ob. said:

That is 58 object leak of TGadgetMetaAccesseor !

This should be easy to track and fix, fixing this part most likely will fix most of the others in that screenshot.

Should it? Bear in mind I'm very new to fixing memory leaks: nothing is obvious to me.

 

6 hours ago, Kas Ob. said:

Where and how these are released, that what you should find and fix

NewAnim doesn't appear to be freed anywhere. However, my attempts to free it have so far only led to program crashes, or the memory leak persisting. Here's TGadgetAnimation's destructor:

destructor TGadgetAnimation.Destroy;
begin
  Dec(fTempBitmapUsageCount);
  if (fTempBitmapUsageCount = 0) then
    fTempBitmap.Free;

  fTriggers.Free;
  fSourceImage.Free;
  fSourceImageMasked.Free;
  inherited;
end;

If I shift-click "inherited" here, we get this empty procedure, in System. So, I assume that either Delphi just somehow 'knows' what to do here, or it does nothing:

destructor TObject.Destroy;
begin
end;

TGadgetMetaAccessor doesn't have a destructor, but TGadgetMetaInfo does. Maybe (i.e. I very much don't know for sure) "Animations.Free" is responsible for dispensing with all created TGadgetAnimations:

destructor TGadgetMetaInfo.Destroy;
var
  i: Integer;
begin
  for i := 0 to ALIGNMENT_COUNT-1 do
  begin
    fVariableInfo[i].Animations.Free;
    fInterfaces[i].Free;
  end;
  inherited;
end;

Shift-clicking "inherited" there leads to the same empty procedure in System.

 

So yes, I'm as baffled as you are.

Edited July 6 by Willicious

[Willicious 8]

OK... this is a bit of a can of worms. fPrimaryAnimation doesn't get freed, and PrimaryAnimation (property referring to fPrimaryAnimation) doesn't get freed. NewInstance and NewAnim also don't get freed. I honestly don't know where to start.

 

The joys of working on someone else's code 🤨

[JonRobertson 72]

48 minutes ago, Willicious said:

fPrimaryAnimation doesn't get freed, and PrimaryAnimation (property referring to fPrimaryAnimation) doesn't get freed. NewInstance and NewAnim also don't get freed. I honestly don't know where to start.

fPrimaryAnimation is likely only a reference to which animation is currently the primary animation.

 

TGadgetAnimations adds NewAnim to a list of some kind (see the calls to Add(NewAnim) and Add(aPrimary). I suspect TGadgetAnimations is responsible for freeing all animations that are added.

 

Is this still SupperLemmix? I downloaded SupperLemmix source just now from https://github.com/Willicious/SuperLemmixPlayer and I do not see TGadgetAnimations in that code.

Edited July 6 by JonRobertson

[JonRobertson 72]

4 minutes ago, JonRobertson said:

I do not see TGadgetAnimations in that code.

Never mind, found it. TGadgetAnimations is a class of TObjectList. TObjectList has a property named OwnsObjects that is True by default. Unless OwnsObjects is set to False, TObjectList will call Free to release all objects that are added to it. See the documentation for TObjectList.OwnsObjects.

 

The only place in the SupperLemmix code where I see OwnsObjects set to False is here:

  fTalismanButtons.OwnsObjects := false; // Because TFLevelSelect itself will take care of any that remain

 

 

[JonRobertson 72]

7 hours ago, Kas Ob. said:

i can't figure out where "GadgetAccessor: TGadgetMetaAccessor;" is freed

GadgetAccessor is assigned by the result of GetInterface. GetInterface creates the TGadgetMetaAccessor and assigns the reference to the FInterfaces array:

function TGadgetMetaInfo.GetInterface(Flip, Invert, Rotate: Boolean): TGadgetMetaAccessor;
var
  i: Integer;
begin
  i := GetImageIndex(Flip, Invert, Rotate);
  if fInterfaces[i] = nil then
    fInterfaces[i] := TGadgetMetaAccessor.Create(self, Flip, Invert, Rotate); <-- Created reference assigned to fInterfaces
  Result := fInterfaces[i];
end;

TGadgetMetaInfo.Destroy loops through fInterfaces and calls Free on each:

destructor TGadgetMetaInfo.Destroy;
var
  i: Integer;
begin
  for i := 0 to ALIGNMENT_COUNT-1 do
  begin
    fVariableInfo[i].Animations.Free;
    fInterfaces[i].Free;
  end;
  inherited;
end;

Despite the misleading names of GetInterface and fInterfaces, these are not Delphi interfaces. TGadgetMetaAccessor is a class derived from TObject.

[JonRobertson 72]

7 hours ago, Kas Ob. said:

as for the UnicodeString leak, it might be simply a confusion/confliction in madshi with MemoryManager

 

16 hours ago, Anders Melander said:

My guess is that it's the TGadgetAnimation.fName string which is leaked (based on the string content and the call to UpperCase) but that it a bit strange because strings are reference counted and it's usually pretty impossible to leak them.

If TGadgetAnimation is not being freed, then it would have a reference to fName and the string would also be a leak.

Edited July 6 by JonRobertson

[Anders Melander 1784]

2 hours ago, Willicious said:

procedure TGadgetAnimations.AddPrimary(aAnimation: TGadgetAnimation);
begin
  Add(aAnimation);
  if fPrimaryAnimation <> nil then
    fPrimaryAnimation.fPrimary := false; <---------------------------------------- this is line 935
  fPrimaryAnimation := aAnimation;
  aAnimation.fPrimary := true;
end;

 

This indicates that fPrimaryAnimation has been destroyed but fPrimaryAnimation has not been nilled.

If you place a breakpoint in the TGadgetAnimation destructor you should be able to see where it is destroyed (Could it possibly be in TGadgetAnimations.Clear ?).

 

if it is TGadgetAnimations itself, or some code that has access to TGadgetAnimations, that destroy the TGadgetAnimation object then you can just have that code clear the fPrimaryAnimation reference. Otherwise TGadgetAnimation will need to store a reference back to the TGadgetAnimations that owns it. This way the TGadgetAnimation destructor can clear the reference when it is destroyed. Something like this:

destructor TGadgetAnimation.Destroy;
begin
  [...]
  // FOwner is a reference to the TGadgetAnimations that contains this object
  if fPrimary then
    FOwner.fPrimaryAnimation := nil; 
  inherited;
end;

 

 

 

[Anders Melander 1784]

1 hour ago, JonRobertson said:

The only place in the SupperLemmix code where I see OwnsObjects set to False is here:

FWIW, it can be set through the constructor too.

 

1 hour ago, JonRobertson said:

If TGadgetAnimation is not being freed, then it would have a reference to fName and the string would also be a leak.

Yes but then I would have expected TGadgetAnimation to be in the leak list too. As far as I can tell from the screenshot the list only contained the string.

Anyway, the string leak is definitely a secondary leak and the problem lies elsewhere.

[JonRobertson 72]

8 minutes ago, Anders Melander said:

FWIW, it can be set through the constructor too.

I also did not see any place where those particular object lists are created with Create(False)

[Willicious 8]

5 hours ago, Anders Melander said:

This indicates that fPrimaryAnimation has been destroyed but fPrimaryAnimation has not been nilled.

If you place a breakpoint in the TGadgetAnimation destructor you should be able to see where it is destroyed (Could it possibly be in TGadgetAnimations.Clear ?).

 

Good shout. Here's the call stack up to the breakpoint:

 

 

It's still not 100% clear how we got to the "Free" call, though... I've highlighted the most recent thing that isn't in System.Generics

[Anders Melander 1784]

Oh, I see... So TGadgetAnimations is in fact declared as derived from TObjectList<TGadgetAnimation>.

 

This means that you can catch the destruction of a TGadgetAnimation by overriding the TObjectList<T>.Notify method:

type
  TGadgetAnimations = class(TObjectList<TGadgetAnimation>)
    [...]
  protected
    procedure Notify(const Value: TGadgetAnimation; Action: TCollectionNotification); override;
    [...]
  end;

procedure TGadgetAnimations.Notify(const Value: TGadgetAnimation; Action: TCollectionNotification); 
begin
  if (Action in [cnExtracting, cnDeleting]) and (Value = fPrimaryAnimation) then
    fPrimaryAnimation := nil;

  inherited;
end;

 

12 minutes ago, Willicious said:

It's still not 100% clear how we got to the "Free" call, though...

The call to Clear destroys all the objects contained in the list. The call stack shows you exactly how you got from TGadgetAnimations.Clear to TGadgetAnimation.Destroy.

[JonRobertson 72]

23 minutes ago, Willicious said:

It's still not 100% clear how we got to the "Free" call, though... I've highlighted the most recent thing that isn't in System.Generics

TGadgetAnimations is a TObjectList that holds a list of TGadgetAnimation:

  TGadgetAnimations = class(TObjectList<TGadgetAnimation>)

There are multiple instances of TGadetAnimations created here:

constructor TGadgetMetaInfo.Create;
var
  i: Integer;
begin
  inherited;
  for i := 0 to ALIGNMENT_COUNT-1 do
  begin
    fVariableInfo[i].Animations := TGadgetAnimations.Create;

The TObjectList.OwnsObjects property defaults to True and is not set to False for any instance of TGadgetAnimations. This means that TGadgetAnimations will call Free on the instances of TGadgetAnimation that it holds (which are added by the call to Add).

 

The TObjectList.Clear method will remove all instances of TGadgetAnimation that were added to that instance of TGadgetAnimations. Since OwnsObjects is True, calling Clear will also call Free on each instance.

 

When AddPrimary is called the first time for the instance, it assigns a reference to TGadgetAnimation to fPrimaryAnimation.

procedure TGadgetAnimations.Clone(aSrc: TGadgetAnimations);
var
  i: Integer;
  NewAnim: TGadgetAnimation;
begin
  Clear;

  NewAnim := TGadgetAnimation.Create(aSrc.PrimaryAnimation.fMainObjectWidth, aSrc.PrimaryAnimation.fMainObjectHeight);
  NewAnim.Clone(aSrc.PrimaryAnimation);
  AddPrimary(NewAnim); <-- This will assign fPrimaryAnimation if it is not assigned.

When Clone is called, it calls Clear, which frees the TGadgetAnimation instances that were previously created. But, it does not clear the fPrimaryAnimation reference, which is now pointing to unallocated memory.

 

Use the solution posted by Anders of overriding the Notify method.

Edited July 6 by JonRobertson

[JonRobertson 72]

1 minute ago, Anders Melander said:

This means that you can catch the destruction of a TGadgetAnimation by overriding the TObjectList<T>.Notify method

Thanks Anders.

 

This approach is should be used rather than the one I posted. 

[Willicious 8]

4 hours ago, Anders Melander said:

The call to Clear destroys all the objects contained in the list. The call stack shows you exactly how you got from TGadgetAnimations.Clear to TGadgetAnimation.Destroy.

Ah yes, I get it now. "Clear;" is called from Clone (as JonRobertson has also pointed out). But, it isn't in the call stack (not directly, anyway). So, (and this is possibly one of those noobish questions), without prior knowledge of where "Clear" is being called, how can the call stack be used to trace this by itself? If the answer is that it can't, that's of course fine - at least we know what we're looking for, so we can search for it initially and obtain the prior knowledge needed to interpret the call stack usefully. Got it.

 

Thanks to you both for all the explanations of what's going on, it's made everything much clearer. I'll go ahead and implement Anders' suggested change and report back what happens.

[Willicious 8]

OK, so Anders' solution gets rid of the built-in resource leak dialog👍, but madExcept still gives us this:

 

[JonRobertson 72]

7 minutes ago, Willicious said:

"Clear;" is called from Clone. But, it isn't in the call stack (not directly, anyway). So, (and this is possibly one of those noobish questions), without prior knowledge of where "Clear" is being called, how can the call stack be used to trace this by itself?

Perhaps it can't without knowledge of the helper class TListHelper. Although you were able to trace back to the point of code where Clear was called and it was shown in your screenshot.

 

 

[Willicious 8]

Well, this is very strange. I went to try madExcept again today, and now the Level Select can't even be accessed:

 

 

With madExcept disabled, the program runs fine and the memory leak is gone. Not sure what's happening here...

[Anders Melander 1784]

19 hours ago, Willicious said:

"Clear;" is called from Clone (as JonRobertson has also pointed out). But, it isn't in the call stack (not directly, anyway).

TList<T>.Clear is declared inline and inlined functions will not appear in the call stack. If you compile with inlining disabled it will appear.

 

19 hours ago, Willicious said:

without prior knowledge of where "Clear" is being called, how can the call stack be used to trace this by itself?

It can't (unless you disable inlining), but in this case we weren't really interested in where Clear was being called from but rather where/when the object was being destroyed.

 

2 hours ago, Willicious said:

I went to try madExcept again today, and now the Level Select can't even be accessed

So, as expected, there's more than one problem. Look at here the Access Violation occurs, examine the call stack, rinse, repeat.

 

2 hours ago, Willicious said:

With madExcept disabled, the program runs fine and the memory leak is gone. Not sure what's happening here...

The absence of errors without madExcept does not mean that aren't any errors. You just can't see them...

[Kas Ob. 121]

10 hours ago, Willicious said:

This access violation is classic corrupted stack, the stack either being overwritten or wrong casting being used with some procedure/function led to not aligned call/ret addresses on the stack.

On sidenote: that address is more like the EXE is not there at all, like it gone, yet the debugger/OS reporting a failed execution on no execution memory address, ... ( it also could be a DLL though).

 

I can't compile the code and don't have experience with MadExcept, but this doesn't sound right at all, and this brings me again to the why MadExcept failed to report the hundreds of leaks and stopped at string with 3 refCount ?

 

There is something wrong with MadExcept or miss configuration in its usage, are you using it with FastMM or any other unit as first use in the dpr file uses clauses ?

[Anders Melander 1784]

1 hour ago, Kas Ob. said:

There is something wrong with MadExcept

Possible but unlikely.

 

I can compile SuperLemmix but I can't run it since I'm missing all the external assets. So instead I went back and looked at the original FastMM leak report.

TGadgetMetaAccessor is owned and destroyed by TGadgetMetaInfo.

TGadgetMetaInfo is created in 3 different places; Two of them are inside TGadgetMetaInfoList (a TObjectList) and those appear to be fine. The last is in TNeoPieceManager where we have a case of exception swallowing that will lead to a leak:

function TNeoPieceManager.ObtainObject(Identifier: String): Integer;
var
  ObjectLabel: TLabelRecord;
  MO: TGadgetMetaInfo;
begin
  try
    ObjectLabel := SplitIdentifier(Identifier);
    Result := fObjects.Count;
    MO := TGadgetMetaInfo.Create;
    MO.Load(ObjectLabel.GS, ObjectLabel.Piece, fTheme); // Leak if exception here
    fObjects.Add(MO);
  except
    Result := -1;
  end;
end;

 

So there's (unsurprisingly) problems with exception handling. With that in mind I searched for "except" and reviewed all the cases of explicit exception handling. Many are misguided but harmless but there are a few serious problems.

For example in TBaseAnimationSet.LoadMetaData there's a double-free in case of exception:

procedure TBaseAnimationSet.LoadMetaData(aColorDict: TColorDict; aShadeDict: TShadeDict);
begin
  Parser := TParser.Create;
  try
    [...]
      try
        [...]
      except
        Parser.Free;
        raise EParserError.Create('TBaseAnimationSet: Error loading lemming animation metadata for ' + ANIM_NAMES[i] + '.')
      end;
    [...]
  finally
    Parser.Free;
  end;
end;

In TNeoLevelEntry.LoadLevelFileData there's a leak on exception:

procedure TNeoLevelEntry.LoadLevelFileData(aExtent: TNeoLevelLoadState);
[...]
            try
              TalInfoLevel.LoadFromFile(Path);
              for i := 0 to TalInfoLevel.Talismans.Count-1 do
              begin
                CloneTal := TTalisman.Create;
                CloneTal.Clone(TalInfoLevel.Talismans[i]); // Leak if exception here
                fTalismans.Add(CloneTal);
              end;
            except
              // Fail silently.
            end;
[...]

TNeoLevelGroup.CleanseLevels is a complete mess and can leak a TStringList on exception.

TNeoLevelGroup.LoadUserData will leak a TParser on exception.