Invalid pointer operation caused by mix of script initilization section and destructor
I'm unsure about the precise cause of the problem. It occurs in a script complex involving eight units and several hundredthousand lines of script code, but I believe the following two units reproduces the problem.
Order of initilization
As far as I have been able to determine the initialization section of the second unit is executed too late. Since the first unit has a dependency on the second unit, the initialization section of the second unit should execute before the first unit. What I'm seeing is that the initialization section of the second unit is executed after the first unit has called a function in the second unit. However, I don't think this is the actual problem.
Clearing references to self during destruction
The TMyStuff
destructor clears an external reference to itself during destruction. Apparently this modifies the reference count of the internal TScriptObjInstance
causing TInterfacedObject.BeforeDestruction
to throw an Invalid pointer operation exception.
Main script file:
unit Test;
interface
uses
Factory;
implementation
initialization
MyStuff;
finalization
end;
Aux script file:
unit Factory;
interface
type
TMyStuff = class
public
destructor Destroy; override;
end;
function MyStuff: TMyStuff;
implementation
var
FMyStuff: TMyStuff = nil;
destructor TMyStuff.Destroy;
begin
FMyStuff := nil;
inherited;
end;
function MyStuff: TMyStuff;
begin
if (FMyStuff = nil) then
FMyStuff := TMyStuff.Create;
Result := FMyStuff;
end;
initialization
FMyStuff := TMyStuff.Create;
finalization
FMyStuff.Free;
end;
Comments (13)
-
repo owner -
repo owner - changed status to resolved
Safer VarClear - Fixed Issue
#25→ <<cset a85ebaebb704>>
-
reporter - changed status to open
I can confirm that your test case no longer reproduces the problem but unfortunately my test case still causes an Invalid pointer operation exception. I suspect there might be more than one problem in play here.
-
repo owner Do you have code in the destructor that acquires Self or copies the instance being destroyed (rather than nil'ing it)?
That's a case that also happens in Delphi, I am not sure yet how to best handle it without patching the rtl.
Code would be like
type TTest = class destructor Destroy; override; end; var ref1, ref2 : TTest; destructor TTest.Destroy; begin ref2 := Self; end; ref1 := TTest.Create; ref1 := nil;
IntfCopy does not check for refcount, so the above code copies the dying reference and increments its reference counter...
-
reporter - edited description
-
reporter No, I do not save the self pointer during destruction.
My actual code is very similar to the test case i posted, except that I only create the instance in the function - not in the initialization section. That said, my actual code produces another error, but I will create another issue for that once I have created a small test case.
Isn't the solution to this issue just that the assignment needs to clear the destination before it assigns the new value?
I.e. this:
Foo := TBar.Create;
should translate to:
var temp := TBar.Create; Foo := nil; Foo := temp;
-
repo owner Which issue would nil'ing before assignment solve?
The first issue comes from an improper nil'ing in the VariantClear function, the second issue is because code that assigns (in IntfCopy) does not check for reference count of the reference before copying and incrementing, so it can increase a refcount from zero back to 1. For instance the following code will crash witha stack overflow in Delphi when TTest is destroyed.
type TTest = class (TInterfacedObject) destructor Destroy; override; end; procedure Boom(ref : IUnknown); begin Assert(ref<>nil); end; destructor TTest.Destroy; begin Boom(Self); end;
Script engine can survive the basic variation of the above (as the destroyed object is getting wrapped), but there may be call chains that cannot be survived.
-
reporter I was referring to the issue that my test case demonstrates.
I'm away from the code so excuse me if I get it wrong but isn't this the sequence that causes the problem (and I'm still referencing my test case):
MyStuff
function performsFMyStuff := TMyStuff.Create
FMyStuff is nil. Create an instance and assign it to FMyStuff.
No problem there.- Initialization section performs
FMyStuff := TMyStuff.Create
FMyStuff is already assigned.
The new assignment causes the destructor to be called before the reference is actually cleared. destructor TMyStuff.Destroy
FMyStuff is still assigned.
Assigning nil to FMyStuff juggles the internal reference count leading to an exception when the internal object is destroyed.
As far as I can tell (and I'm just guessing here) if an assignment internally was performed as a clear followed by a copy, then step 2 above would not cause the problem.
-
repo owner Ah, sorry, I had not seen it under that angle, I will look at it!
-
repo owner Ok spotted the issue, it's a mix of the two: VarCopy is doing a VarClear before the assignment (like in your suggestion), but using VariantClear, which does not nil the reference before calling _Release...
Variant copies in the code are hidden behind simple ":=" assignments, so hard to track them all to have them replace by explicit calls...
-
repo owner Committed a fix for the above case and hopefully some variant, the issue still likely exists in other cases.
-
reporter Excellent!
I can confirm that the test case no longer produces the problem. -
repo owner - changed status to closed
Closing it for now, but phasing out all VarCopy calls I can find (explicit & implicit)
- Log in to comment
I could reproduce it with the following minimal test case.
I could also reproduce the problem in Delphi for an Interface in a Variant: VarClear calls VariantClear (Windows API) which calls _Release before nil'ing the reference, so upon the next assignment to nil, it is called again, resulting in an invalid refcount.
IntfClear does it the other way (nil before calling _Release), which avoids the issue. I guess this means the VarClear calls to have variant that does not rely on the Windows API.