Watch, Follow, &
Connect with Us
Public Report
Report From: Delphi-BCB/Compiler/Delphi/Interfaces    [ Add a report in this area ]  
Report #:  90482   Status: Closed
The compiler should keep a hidden reference when passing freshly created object instances directly as const interface parameters
Project:  Delphi Build #:  15.0.3890.34076
Version:    15.0 Submitted By:   Jeroen Pluimers
Report Type:  Crash / Data loss / Total failure Date Reported:  12/22/2010 3:13:31 PM
Severity:    Commonly encountered problem Last Updated: 7/18/2014 12:37:06 AM
Platform:    All platforms Internal Tracking #:   281509
Resolution: As Designed (Resolution Comments) Resolved in Build: : None
Duplicate of:  None
Voting and Rating
Overall Rating: (3 Total Ratings)
5.00 out of 5
Total Votes: 65
Description
As per Barry Kelly: this is a compiler bug.

It started out as
http://stackoverflow.com/questions/4509015/should-the-compiler-hint-warn-when-passing-object-instances-directly-as-const-int/4510268#4510268
and the comments there lead to a set of test cases showing which permuations are OK, and which are not.

That means this QC/RAID issue now encompasses these other QC/RAID issues:
- QC 31164
- QC 71015/RAID 268053
- QC 75036/RAID 270259

I found the bugs below while finally deeply researching some spurious crashes in large projects I was involved with over time.
It has been present in the Delphi compiler for a long time (at least since 5, but likely since interfaces were introduced).

The bug manifests it in two ways:
- if you always use const interface parameters in a method chain, the underlying object instance never gets freed at all
- if you have just one spot in the method chain that performs _AddRef/_Release, the first occurance of that frees the underlying object; consecutive references than will get you undermined results (depending on the settings of your memory manager)

Reproducing the test-cases in the attachments can be best done with these conditional defines:
- {$define FastMM}
- {$define EnableMemoryLeakReporting}
- {$define FullDebugMode}
- {$define FullDebugModeCallBacks}
- {$define CatchUseOfFreedInterfaces} // disables CheckUseOfFreedBlocksOnShutdown

The spuriousnes lies in the fact that the projects are multi-developer, where some developers like const-parameters where others like value-parameters.
So sometimes parameters change from const to value or vice versa.
Those changes make the issue go away or introduce it at seemingly randomg places, especially when the methods and call places are far apart.
It also depends under which circumstances the Delphi compiler actually adds _AddRef/_Release calls (this has changed over time, hence for instance the difference in Delphi XE).

Currently, when passing a freshly created object instance (TMyClass.Create) directly as a const-parameter of an interface type, then the first reference counted usage of that parameter will destroy the underlying object instance.
This is because the const parameter does not perform reference counting, and since the paramter is directly passed, it starts with a reference count of zero.
The result is at the second reference counted usage of the parameter: since the underlying object has now been destroyed, bad things happen.
If no reference counted usage takes place, then you have a memory leak.
Both cases are wrong.

This seems like a corner case, but in practice it is not: lots of people pass around freshly created objects without storing them in a termporary interface field or variable.
Lots of code accepts interface parameters, often as const parameters.
Interfaces are used more often than in the past, so the chance of bumping into these issues

The problem in a nutshell manifests itself by calling RunPrematureFreeCrash in this code; it raises EInvalidPointer, but shouldn't:

<<
procedure ReferTo(ValueReference: IUnknown);
begin
// remove the comments to reproduce it in earlier Delphi versions (I reproduced it in Delphi 5, 2007, 2009 and XE)
//  if not Assigned(ValueReference) then
//    ;
end;

procedure PrematureFreeCrash(const Reference: IUnknown);
begin
  ReferTo(Reference);
  ReferTo(Reference); // the second call will crash inside ReferTo because the object instance that Reference referes to just got destroyed
end;

procedure RunPrematureFreeCrash;
begin
  PrematureFreeCrash(TInterfacedObject.Create);
end;
>>

The attached test-case will reproduce it; it contains more permutations of const/value, local reference, etc in order for me to focus down on the actual issue.
The stackoverflow question also contains reproducible source, with augmented writeln statements to view what happens under the hood.

From the attachment, you only need the InterfaceConstParmetersAndPrematureFreeingTestCaseUnit.pas, but the rest of the project gives you more context.

The solution is quite simple: The compiler should keep a hidden reference when passing freshly created object instances directly as const interface parameters.
This solves both manifestations of the bug:
- underlying object instances won't be destroyed too early (because of the reference)
- underlying object instances will be destroyed (because of the reference will get an _AddRef and finally _Release)

--jeroen

Steps to Reproduce:
A short (but incomplete) reproduction is to call RunPrematureFreeCrash in this code; it raises EInvalidPointer, but shouldn't.

A more comlete reproduction (containing many more permutations) is in the attachment.

<<
procedure ReferTo(ValueReference: IInterface);
begin
end;

procedure PrematureFreeCrash(const Reference: IInterface);
begin
  ReferTo(Reference);
  ReferTo(Reference); // the second call will crash inside ReferTo because the object instance that Reference referes to just got destroyed
end;

procedure RunPrematureFreeCrash;
begin
  PrematureFreeCrash(TInterfacedObject.Create);
end;
>>

Workarounds
Don't pass such an instance; always keep a local interface reference to it before passing it on.
Attachment
InterfaceConstParmetersAndPrematureFreeing.zip
Comments

David Heffernan at 12/23/2010 2:34:05 AM -
This behaviour has certainly been present since Delphi 6 (I have Delphi 6 projects that work around it).

Chee Yang Chau at 6/7/2011 6:22:58 PM -
Try this and the leak gone:

procedure RunPrematureFreeCrash;
begin
  PrematureFreeCrash(TInterfacedObject.Create as IInterface);
end;

Tomohiro Takahashi at 7/18/2014 12:37:32 AM -
This is a comment from internal tracking system.
<<<
Works as expected.
Should not pass const interface parameter to non-const parameter.
>>>

Server Response from: ETNACODE01