Constant string parameters and the Delphi XE5 compiler for Android

Question

Hopefully I'm just missing something obvious, but I seem to be finding constant string arguments getting corrupted when using the Delphi XE5 Android compiler. Test code:

1) Create a new blank mobile application project.

2) Add a TButton to the form, and create an OnClick handler for it.

3) Fill out the handler like so:

procedure TForm1.Button1Click(Sender: TObject);
begin
  GoToDirectory(PathDelim + 'alpha' + PathDelim + 'beta');
  GoToDirectory(FParentDir);
end;

4) In the form class declaration, add two fields and one method like this:

FCurrentPath, FParentDir: string;
procedure GoToDirectory(const Dir: string);

5) Implement Foo and GoToDirectory like so:

function Foo(const S: string): Boolean;
begin
  Result := (Now <> 0);
end;

procedure TForm1.GoToDirectory(const Dir: string);
begin
  FCurrentPath := IncludeTrailingPathDelimiter(Dir);
  FParentDir := ExcludeTrailingPathDelimiter(ExtractFilePath(Dir));
  ShowMessageFmt('Prior to calling Foo, Dir is "%s"', [Dir]);
  Foo(FParentDir);
  ShowMessageFmt('After calling Foo, Dir is "%s"', [Dir]);
end;

6) Compile and run on a device.

When I do this, the first two message boxes don't indicate anything wrong, however Dir then gets cleared in between the third and fourth prompts. Does anyone else get this, or am I just doing something silly? (There is nothing untoward when I target Win32 for testing purposes.)

Update

For a FMX-free version, create a new blank mobile application again, but this time remove the form from the project. Then, go into the project source and add the following code:

program Project1;

uses
  System.SysUtils,
  Androidapi.Log;

type
  TTest = class
  private
    FCurrentPath, FParentDir: string;
    procedure GoToDirectory(const Dir: string);
  public
    procedure Execute;
  end;

function Foo(const S: string): Boolean;
begin
  Result := (Now <> 0);
end;

procedure TTest.GoToDirectory(const Dir: string);
var
  M: TMarshaller;
begin
  FCurrentPath := IncludeTrailingPathDelimiter(Dir);
  FParentDir := ExcludeTrailingPathDelimiter(ExtractFilePath(Dir));

  LOGE(M.AsUtf8(Format('Prior to calling Foo, Dir is "%s"', [Dir])).ToPointer);
  Foo(FParentDir);
  LOGE(M.AsUtf8(Format('After to calling Foo, Dir is "%s"', [Dir])).ToPointer);
end;

procedure TTest.Execute;
begin
  GoToDirectory(PathDelim + 'alpha' + PathDelim + 'beta');
  GoToDirectory(FParentDir);
end;

var
  Test: TTest;
begin
  Test := TTest.Create;
  Test.Execute;
end.

To see the result, first run monitor.bat in the Android SDK tools folder; to see the wood through the trees, filter only for errors given I've used LOGE calls. While not every time I run this revised test app the argument gets corrupted, it does still sometimes... which is indicating a rather nasty compiler bug...

Update 2

With the second test case especially I'm convincing myself even more, so I've logged it as QC 121312.

Update 3

A code rather than prose version of the explanation in the accepted answer below (interface types using essentially the same reference counting mechanism as strings, only with the ability to easily track when the object is destroyed):

program CanaryInCoalmine;

{$APPTYPE CONSOLE}

uses
  System.SysUtils;

type
  ICanary = interface
    function GetName: string;
    property Name: string read GetName;
  end;

  TCanary = class(TInterfacedObject, ICanary)
  strict private
    FName: string;
    function GetName: string;
  public
    constructor Create(const AName: string);
    destructor Destroy; override;
  end;

  TCoalmine = class
  private
    FCanary: ICanary;
    procedure ChangeCanary(const Arg: ICanary);
  public
    procedure Dig;
  end;

constructor TCanary.Create(const AName: string);
begin
  inherited Create;
  FName := AName;
  WriteLn(FName + ' is born!');
end;

destructor TCanary.Destroy;
begin
  WriteLn(FName + ' has tweeted its last song');
  inherited;
end;

function TCanary.GetName: string;
begin
  Result := FName;
end;

procedure TCoalmine.ChangeCanary(const Arg: ICanary);
var
  OldName: string;
begin
  Writeln('Start of ChangeCanary - reassigning FCanary...');
  OldName := Arg.Name;
  FCanary := TCanary.Create('Yellow Meanie');
  Writeln('FCanary reassigned - is ' + OldName + ' still alive...?');
  Writeln('Exiting ChangeCanary...');
end;

procedure TCoalmine.Dig;
begin
  FCanary := TCanary.Create('Tweety Pie');
  ChangeCanary(FCanary);
end;

var
  Coalmine: TCoalmine;
begin
  Coalmine := TCoalmine.Create;
  Coalmine.Dig;
  ReadLn;
end.

The output is this:

Tweety Pie is born!
Start of ChangeCanary - reassigning FCanary...
Yellow Meanie is born!
Tweety Pie has tweeted its last song
FCanary reassigned - is Tweety Pie still alive...?
Exiting ChangeCanary...

As such, reassigning the field drops the reference count of the previous object, which given there is no other strong reference to it, destroys it there and then before the ChangeCanary procedure has finished.

Answer 1

We did some internal investigations, and it turns out this depends on the way the code is written and there is nothing the compiler can really do about it. It is slightly complex, but in short the GoToDirectory method receives a const string parameter (Dir) which refers to a string. However, within the method's code replaces the string with a new one (which might be at the same or at a different memory location). Given the const parameter doesn't increase the reference count, if you decrease the ref count of the same string in the code, the string is removed. So you have a parameter pointing to an undefined memory location, and the actual output is kind of random. Same issue happens (might happen) on all platforms, not mobile specific.

There are many workarounds:

1) not have the const param (so the ref count is higher, you change the referenced string but the param is now a reference to a separate string

2) Pass an alias of the string:

  Tmp := FParentDir;
  GoToDirectory(Tmp);

3) Assign "const String" parameter to temporary local variable:

procedure TForm1.GoToDirectory(const Dir: string);
var
  TmpDir: String;
begin
  TmpDir := Dir;

I know this is far from a clear description, and I had to red it a few times to grasp it, but it is a scenario the compiler cannot really handle automatically, so we are going to close the bug report "as designed".

Answer 2

To expand on Marco's comment a bit, this pit-fall on the use of const on a parameter has been in Delphi since the introduction of const parameters and is not a bug but, rather, a feature that your example is an example of a case it shouldn't be used.

The const modifier is a promise to the caller that there is no way that a variable passed as a parameter will be modified as a side-effect of the call. The simplest way to guarantee this is to never modify a globally accessible variable in a function or procedure with a const parameter. This allows the callee to rely on the caller's reference count, avoid copy semantics, etc. In other words, it tells the compiler that if the value is more efficiently passed as a var and it can be treated as a var parameter (that is, has an lvalue) then pass it as a var instead of a value. If it is a managed type, like a string, it can also rely on the caller's reference to keep the memory alive.

This contract is violated by GoToDirectory when it modifies a global accessible string (any heap access should be considered global, in this context, even though it is a field of an object). GoToDirectory should not have a const parameter because it violates the contract implied by const.

Note that this differs significantly than the contract implied by const in other languages, such as C++. It is unfortunate that there wasn't a better word to use at the time. What it really is saying is that the function or procedure is pure with respect to variables compatible with the formal type of the const argument, not that it will not modify the argument. It is easier just to remember, don't apply const to any parameter of a function or procedure that has a side-effect.

That rule-of-thumb can be violated when the side-effect of the write to a global will not be visible to the procedure or function or any procedures or functions it calls. That is usually very hard to guarantee outside trivial cases (such as a simple property setter) and should only be used if a performance constraint cannot afford the overhead of the value copy. In other words, you better have performance trace in hand to justify it or it better be obvious to the casual observer that it a copy would be expensive.