Advice on "Invalid Pointer Operation" when using complex records

Question

Env: Delphi 2007

<Justification>I tend to use complex records quite frequently as they offer almost all of the advantages of classes but with much simpler handling.</Justification>

Anyhoo, one particularly complex record I have just implemented is trashing memory (later leading to an "Invalid Pointer Operation" error).

This is an example of the memory trashing code:

sSignature := gProfiles.Profile[_stPrimary].Signature.Formatted(True);

On the second time i call it i get "Invalid Pointer Operation"

It works OK if i call it like this:

  AProfile    := gProfiles.Profile[_stPrimary];
  ASignature  := AProfile.Signature;
  sSignature  := ASignature.Formatted(True);

Background Code:

  gProfiles: TProfiles;

  TProfiles = Record
  private
    FPrimaryProfileID: Integer;
    FCachedProfile: TProfile;
    ...
  public
    < much code removed >

    property Profile[ProfileType: TProfileType]: TProfile Read GetProfile;
  end;


  function TProfiles.GetProfile(ProfileType: TProfileType): TProfile;
  begin        
    case ProfileType of
      _stPrimary        : Result := ProfileByID(FPrimaryProfileID);
      ...
    end;
  end;

  function TProfiles.ProfileByID(iID: Integer): TProfile;
  begin
    <snip>
    if LoadProfileOfID(iID, FCachedProfile)  then
    begin
      Result := FCachedProfile;
    end
    else
    ...
  end;


  TProfile = Record
  private     
    ...
  public
    ...
    Signature: TSignature;
    ...
  end;


  TSignature = Record
  private               
  public
    PlainTextFormat : string;
    HTMLFormat      : string;

    // The text to insert into a message when using this profile
    function Formatted(bHTML: boolean): string;
  end;

  function TSignature.Formatted(bHTML: boolean): string;
  begin
    if bHTML then
      result := HTMLFormat
    else
      result := PlainTextFormat;
    < SNIP MUCH CODE >
  end;

OK, so I have a record within a record within a record, which is approaching Inception level confusion and I'm the first to admit is not really a good model. Clearly i am going to have to restructure it. What I would like from you gurus is a better understanding of why it is trashing the memory (something to do with the string object that is created then freed...) so that i can avoid making these kinds of errors in future.

Thanks

Answer 1

Your justification for using records over classes seems flawed. Every time you return a record as a function result or pass a record as a function parameter or assign from one record var to another record var, all the fields of that record structure are being copied in memory.

This alone can be cause for concern. Compared to reference types, passing record type variables around can suck the life out of a program. Your code could easily spend more time copying stuff from here to there and there to here than on actually getting work done.

The difference between calling your three functions in series in one statement versus calling the three functions in separate statements is the allocation and lifetime of the intermediate results. When you call the functions in separate statements, you provide local variables to hold the intermediate results between calls. The variables are explicit and their lifetimes are well defined.

When you call the functions in one statement, the compiler is responsible for allocating temporary variables to hold the intermediate results between calls. Lifetime analysis of these implicit variables can become murky - can the same local variable be used to hold the intermediate results of multiple calls in series? Most of the time, the answer is probably yes, but if the record types involved contain fields of compiler-managed data types (strings, variants, and interfaces) the same local variable can't just be overwritten with the next block of data.

Records containing compiler-managed types must be disposed of in an orderly fashion to avoid leaking heap memory. If such a record is overwritten with garbage data, or if such a record is copied without the compiler's awareness, then the compiler generated code to dispose of the compiler-managed fields of the record when the record goes out of scope will likely report that it encountered an invalid pointer and/or a corrupt heap.

Your TSignature record contains string fields, making it a compiler-managed data type. Everywhere that you have a local variable of type TSignature, the compiler has to implicitly generate try..finally frames in the body of the function to make sure the string fields in that local variable structure get released when execution leaves that scope.

Any operation that ends up modifying or overwriting the string field pointers in the TSignature record can lead to an Invalid Pointer Operation error. Making copies of the record (by assigning it to multiple variables) should increment the ref counts automatically, but any use of MemCopy to bulk copy the record's contents to some other location will throw off the ref counts and lead to Invalid Pointer Operation when the cleanup code tries to release those string fields more times than they were actually referenced. Typecasting a record variable to the wrong record type could cause the string fields to be overwritten with garbage, and cause an Invalid Pointer Operation down the line (when the record is cleaned up at the end of the scope)

There is also a possibilty that the compiler itself has lost track of the intermediate record variables in the single-statement scenario and is cleaning up the hidden intermediates too many times or overwriting them without cleaning up the prior values. There was a compiler bug in this area somewhere back in the Delphi 3 era, but I don't recall which product release we fixed it in. I seem to recall the bug I have in mind involved record type function results being passed to const type parameters, so it's not an exact match to your scenario, but the consequences are similar.

Before reporting this as a compiler bug, go over your code with a fine-toothed comb in the debugger disassembly view. There are tons of ways you can screw this up all by yourself. See where the intermediate results are being allocated, written to, and disposed by the compiler-generated code, and how your code is interacting with that pattern.

The smoking gun will be when you see the string fields of one temp record variable overwritten without a call to decrement the reference to those strings. It could be caused by your code, or it could be caused by something in the compiler generated code, but the only way to find out is to witness the act and work out the finger pointing from there.

Answer 2

Its not apparent from the code you supplied where the corruption is happening so here are a few suggestions. Try different combinations of field chaining to see if you can reproduce it.

AProfile := gProfiles.Profile[_stPrimary];
sSignature := AProfile.Signature.Formatted(True);

ASignature := gProfiles.Profile[_stPrimary].Signature;
sSignature := ASignature.Formatted(True);

Turn on range checking and overflow checking if you haven't already. Download FastMM4 and use its FullDebugMode. If none of this leads to an answer learn how to use memory breakpoints.

Answer 3

There is something I don't get very well with your code extract. Is TProfile a record? So using a function SomeName: TProfile will make a copy of the record content into the result, which is very inefficient. Even with an optimized version of the record copy function, it's still time consuming.

You should get it by reference/pointer, using a PProfile = ^TProfile type. In this case, you'll prevent most of your memory problems, about accessing string inside a record.

But you should be sure than your original TProfile will remain available in memory, during the whole lifetime of the PProfile pointer.

Using records can be faster/easier than using classes in some (rare) cases, if you parse some binary content for example. But you should never use a plain record type to manipulate records with functions/methods, but a pointer to a record (or a var parameter). It will be both safer and faster.