Sigsegv with refcounting interface

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Sigsegv with refcounting interface

Joao Morais-2
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.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

José Mejuto
El 07/03/2013 12:29, Joao Morais escribió:
> 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;

--

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

Joao Morais-2
On Thu, Mar 7, 2013 at 9:27 AM, José Mejuto <[hidden email]> wrote:
>
> 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.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

etrusco
>
> 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
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

Joao Morais-2
On Fri, Mar 8, 2013 at 4:09 PM, Flávio Etrusco <[hidden email]> 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.

> 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
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

Jonas Maebe-2

On 09 Mar 2013, at 02:37, Joao Morais wrote:

On Fri, Mar 8, 2013 at 4:09 PM, Flávio Etrusco <[hidden email]> 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

_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

Joao Morais-2
On Sat, Mar 9, 2013 at 8:33 AM, Jonas Maebe <[hidden email]> wrote:

>
> On 09 Mar 2013, at 02:37, Joao Morais wrote:
>>
>> On Fri, Mar 8, 2013 at 4:09 PM, Flávio Etrusco <[hidden email]>
>> 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.
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal
Reply | Threaded
Open this post in threaded view
|

Re: Sigsegv with refcounting interface

Joao Morais-2
Sorry about breaking the thread.

On Sun, Mar 10, 2013 at 3:47 PM, Joao Morais <[hidden email]> wrote:
> 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
_______________________________________________
fpc-pascal maillist  -  [hidden email]
http://lists.freepascal.org/mailman/listinfo/fpc-pascal