Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-pascal / 2013-03

Sigsegv with refcounting interface

8 messages · 4 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Joao Morais

Hello list, what's the problem with "RoundThree" procedure (below)? Here it raises a sigsegv trying to read an internal field. The difference from "RoundTwo" is that I create an implementation of the interface within the first param of tinterfacedlist.add method.

Same problem with fpc 2.6.2 and 2.7.1 (from two days ago), i386-win32.

Joao Morais

==============

program project1;
{$mode objfpc}{$H+}

uses
heaptrc,
sysutils,

  Classes;
type
  iintf = interface
  ['{9E08FFD2-5AC4-4AE7-B2C6-703D62A10F16}']
    function RefCount: Integer;
  end;
  { tintf }
  tintf = class(TInterfacedObject, iintf)
  public
    constructor Create;
    destructor Destroy; override;
    function RefCount: Integer;
  end;

{ tintf }

constructor tintf.Create;
begin
  inherited Create;
  writeln('tintf "', PtrUInt(Self) ,'" created');
end;
destructor tintf.Destroy;
begin
  writeln('tintf "', PtrUInt(Self) ,'" destroyed');
  inherited Destroy;
end;
function tintf.RefCount: Integer;
begin
  Result := inherited RefCount;
end;
procedure RoundOne;
var
  vintf: TInterfaceList;
  v1: iintf;
begin
  writeln;
  writeln('Round one');
  writeln('---------');
  vintf := TInterfaceList.Create;
  try
    writeln('creating and adding...');
    vintf.Add(tintf.create);
    writeln('retrieving to v1...');
    if vintf[0].QueryInterface(iintf, v1) = S_OK then
    begin
      writeln('trying to write v1.RefCount...');
      writeln('counting: ', v1.RefCount);
    end else
      writeln('problem');
    v1 := nil;
  finally
    FreeAndNil(vintf);
  end;
end;
procedure RoundTwo;
var
  vintf: TInterfaceList;
  v1, v2: iintf;
begin
  writeln;
  writeln('Round two');
  writeln('---------');
  vintf := TInterfaceList.Create;
  try
    writeln('creating...');
    v2 := tintf.Create;
    writeln('adding...');
    vintf.Add(v2);
    writeln('typecasting to v1...');
    v1 := iintf(vintf[0]);
    writeln('trying to write v1.RefCount...');
    writeln('counting: ', v1.RefCount);
    v1 := nil;
    v2 := nil;
  finally
    FreeAndNil(vintf);
  end;
end;
procedure RoundThree;
var
  vintf: TInterfaceList;
  v1: iintf;
begin
  writeln;
  writeln('Round three');
  writeln('-----------');
  vintf := TInterfaceList.Create;
  try
    writeln('creating and adding...');
    vintf.Add(tintf.Create);
    writeln('typecasting to v1...');
    v1 := iintf(vintf[0]);
    writeln('trying to write v1.RefCount...');
    writeln('counting: ', v1.RefCount);
    v1 := nil;
  finally
    FreeAndNil(vintf);
  end;
end;
begin
  RoundOne;
  RoundTwo;
  RoundThree;
end.

Reply by email

José Mejuto
to Joao Morais
Joao Morais wrote 7 lines
Hello list, what's the problem with "RoundThree" procedure (below)? Here it raises a sigsegv trying to read an internal field. The difference from "RoundTwo" is that I create an implementation of the interface within the first param of tinterfacedlist.add method. writeln('typecasting to v1...'); v1 := iintf(vintf[0]);

Hello,

AFAIK you are casting a IUnknown to iintf instead requesting the iintf interface. I think the right procedure should be:

v1 := vintf[0] as iintf;

Reply by email

Joao Morais
to José Mejuto
José Mejuto wrote 7 lines
Hello, AFAIK you are casting a IUnknown to iintf instead requesting the iintf interface. I think the right procedure should be: v1 := vintf[0] as iintf;

Hello, thanks for your reply.

You are right, changing to a safe typecast, don't know exactly why, works flawlessly. Anyway the following code reproduce the exact problem I have here. The 'sucess!' message isn't called.

Using 2.6.2 and 2.7.1 (2013/03/03)

Joao Morais

=============

program project1;
{$mode objfpc}{$H+}

uses
heaptrc,
sysutils,

  Classes;
var
  vintfl: TInterfaceList;
procedure addintf(const aintf: IUnknown);
begin
  if vintfl.IndexOf(aintf) = -1 then
    vintfl.Add(aintf);
end;
begin
  vintfl := TInterfaceList.Create;
  try
    writeln('starting...');
    addintf(TInterfacedObject.Create);
    writeln('success!');
  finally
    FreeAndNil(vintfl);
  end;
end.

Reply by email

Flávio Etrusco
to Joao Morais
var vintfl: TInterfaceList;

BTW You should use a IInterfaceList variable

procedure addintf(const aintf: IUnknown); (...) addintf(TInterfacedObject.Create); (...)

It's a pity, but I don't know whether it's fixable (or if needs to be "fixed") in the compiler.
The TInterfacedObject has RefCount = 0 after construction, so you should always to assign it to a variable.
In a sense a RefCounted object only exists while someone has a reference to it, so in that case the object non-existant ;-)

-Flávio

Reply by email

Joao Morais
to Flávio Etrusco
Flávio Etrusco wrote 8 lines
> procedure addintf(const aintf: IUnknown); > (...) > addintf(TInterfacedObject.Create); > (...) It's a pity, but I don't know whether it's fixable (or if needs to be "fixed") in the compiler.

Works in Delphi 7 with FastMM4.

The TInterfacedObject has RefCount = 0 after construction, so you should always to assign it to a variable. In a sense a RefCounted object only exists while someone has a reference to it, so in that case the object non-existant ;-)

Correct. I am using an interface reference -- in the indexOf() method in my sample -- before its "definitive persistence" as an intflist member, and without a "real" variable that holds the instance alive because of the const parameter.

You are suggesting that this unlikely coincidente which causes a sigsegv should be the programmer's responsibility. No thanks, I much more prefer the Delphi compatibility =)

Joao Morais

Reply by email

Jonas Maebe
to Joao Morais
Joao Morais wrote 11 lines
On Fri, Mar 8, 2013 at 4:09 PM, Flávio Etrusco wrote: > >> procedure addintf(const aintf: IUnknown); >> (...) >> addintf(TInterfacedObject.Create); >> (...) > > It's a pity, but I don't know whether it's fixable (or if needs to be > "fixed") in the compiler. Works in Delphi 7 with FastMM4.

[...]

You are suggesting that this unlikely coincidente which causes a sigsegv should be the programmer's responsibility. No thanks, I much more prefer the Delphi compatibility =)

It indeed only works by accident in your Delphi code. It is just as wrong in Delphi as it is in FPC. It depends on an implementation detail of the code generator's reference counting. It's like saying that a particular piece of code in which an uninitialized local variable happens to be always 0 in Delphi should have the same behaviour in FPC. We don't aim for compatibility as far as providing the same behaviour for code of which the result is undefined.

Jonas

Reply by email

Joao Morais
to Jonas Maebe
Jonas Maebe wrote 14 lines
On 09 Mar 2013, at 02:37, Joao Morais wrote: > > On Fri, Mar 8, 2013 at 4:09 PM, Flávio Etrusco > wrote: >> >> [...] >> > > You are suggesting that this unlikely coincidente which causes a > sigsegv should be the programmer's responsibility. No thanks, I much > more prefer the Delphi compatibility =) It indeed only works by accident in your Delphi code.

I see your point and this helped me find what's helping delphi work by accident, thanks for that. It's nice to see that the code bellow also fails when compiled with delphi. =)

We don't aim for compatibility as far as providing the same behaviour for code of which the result is undefined.

I fully agree with you: code which results in an undefined behavior is a programmer's responsibility. I was talking about some refcounting management which doesn't exist. It's clear now that this scenario is like memory leak because of circular references: another issue of refcounted objects.

Anyway I'd like to see some fixes in the rtl, not because of compatibility but because of performance improvement: not declaring refcounted parameters such as ansi strings and com interfaces will lead to useless calls to incref and decref. Should I open a bug report for this improvement for some popular classes (ie this may be changed in thunk)?

Joao Morais

=====================

program project1;
{$ifdef fpc}{$mode objfpc}{$H+}{$endif}
// declare "breakcode" to break the code =)
{.$define breakcode}
uses
  {$ifdef fpc}heaptrc{$else}fastmm4{$endif},
  sysutils, Classes;
{$ifndef fpc}
type
  PtrUInt = longint;
{$endif}
procedure stuff({$ifndef breakcode}const{$endif} v: IUnknown);
begin
  writeln('current: ', PtrUInt(v));
end;
var
  vintfl: TInterfaceList;
procedure addintf(const aintf: IUnknown);
begin
  writeln('calling stuff...');
  stuff(aintf);
  writeln('calling indexOf');
  if vintfl.IndexOf(aintf) = -1 then
  begin
    writeln('adding to intflist...');
    vintfl.Add(aintf);
  end;
end;
begin
  vintfl := TInterfaceList.Create;
  try
    writeln('starting...');
    addintf(TInterfacedObject.Create);
    writeln('success!');
  finally
    FreeAndNil(vintfl);
  end;
end.

Reply by email

Joao Morais
to Joao Morais

Sorry about breaking the thread.

Joao Morais wrote 6 lines
Anyway I'd like to see some fixes in the rtl, not because of compatibility but because of performance improvement: not declaring refcounted parameters such as ansi strings and com interfaces will lead to useless calls to incref and decref. Should I open a bug report for this improvement for some popular classes (ie this may be changed in thunk)?

not declaring refcounted parameters AS CONST, such as ansi strings...

Joao Morais

Reply by email