View Issue Details

IDProjectCategoryView StatusLast Update
0026602FPCCompilerpublic2018-10-04 16:10
Reporterstocki Assigned ToJonas Maebe  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionno change required 
Product Version2.7.1 
Summary0026602: Serious bug in TInterfacedObject / IUnknown
Descriptionprogram Bug;

{$APPTYPE console}

uses
  sysutils, classes;

type
  TIntfFoo = class(TInterfacedObject, IUnknown)
  public
    destructor Destroy; override;
  end;

destructor TIntfFoo.Destroy;
begin
  writeln('-->Destroy: You should not see this in console!');
  inherited;
end;

function Interf: IUnknown;
begin
  Result := TIntfFoo.Create;
end;

{
Interfaced objects Destroy is called *before* procedure goes out of scope!

FPC:
-->Start
-->Destroy: You should not see this in console!
-->End

Delphi:
-->Start
-->End
}

begin
  writeln('-->Start');
  Interf;
  writeln('-->End');
  readln;
end.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Relationships

related to 0030409 resolvedMichael Van Canneyt FPC delphi mode behaviour does not honour Delphi documented object lifetimes 

Activities

Do-wan Kim

2014-08-17 03:09

reporter   ~0076618

Last edited: 2014-08-17 05:22

View 2 revisions

delphi bug?

0040164A e801b10000 call 0x40c750 <fpc_get_output>
0040164F 89c3 mov %eax,%ebx
00401651 b9cc504200 mov $0x4250cc,%ecx
00401656 89da mov %ebx,%edx
00401658 b800000000 mov $0x0,%eax
0040165D e8ceb20000 call 0x40c930 <fpc_write_text_shortstr>
00401662 e899820000 call 0x409900 <fpc_iocheck>
00401667 89d8 mov %ebx,%eax
00401669 e822b20000 call 0x40c890 <fpc_writeln_end>
0040166E e88d820000 call 0x409900 <fpc_iocheck>
interface_ins.lpr:40 Interf;
00401673 8d45fc lea -0x4(%ebp),%eax
00401676 e865ffffff call 0x4015e0 <INTERF>
0040167B 8d45fc lea -0x4(%ebp),%eax // interface in eax
0040167E e8cd6f0000 call 0x408650 <fpc_intf_decr_ref> // free object instance.
interface_ins.lpr:41 writeln('-->End');


under linux-64. there is no difference.

00000000004002F2 4889de mov %rbx,%rsi
00000000004002F5 bf00000000 mov $0x0,%edi
00000000004002FA e8c1b70100 callq 0x41bac0 <fpc_write_text_shortstr>
00000000004002FF e8cc590100 callq 0x415cd0 <fpc_iocheck>
0000000000400304 4889df mov %rbx,%rdi
0000000000400307 e8e4b60100 callq 0x41b9f0 <fpc_writeln_end>
000000000040030C e8bf590100 callq 0x415cd0 <fpc_iocheck>
intftest.lpr:40 Interf;
0000000000400311 488d7d98 lea -0x68(%rbp),%rdi
0000000000400315 e846ffffff callq 0x400260 <INTERF>
000000000040031A 488d7d98 lea -0x68(%rbp),%rdi
000000000040031E e81d100100 callq 0x411340 <fpc_intf_decr_ref>
intftest.lpr:41 writeln('-->End');
0000000000400323 e8e8b40100 callq 0x41b810 <fpc_get_output>

Thaddy de Koning

2014-08-17 03:57

reporter   ~0076619

Last edited: 2014-08-17 07:57

View 10 revisions

Can't reproduce (edit, I can but not 2.7.1 windows). Both 2.7.1 and D7 under windows give the correct result.
-->Start
-->End

-->Destroy: You should not see this in console!

About the"you should not...": That's not true. The console buffer (which isn't part of the process, but part of the console, windows first starts a console and then the console application as its child.) is filled during clean-up and there is a pending readln on exit. That's where the dump - in the sense of the screen write - comes from. The console simply dumps after exit and this is out of process. The console still is signaled. Control to the console is transferred after program cleanup is finished and so will dump its buffer.
You can't test this sort of thing in the console. Not with readln/writeln.

EDIT:
Under 2.6 debian I get

-->Start
-->Destroy: You should not see this in console!
-->End

Which is unexpected.

Seems the console is not transacted under Linux?
Under windows, the console cleans up the buffer and the signals, not the program. Under Linux it may be the other way around?

Anyway the line should always be written.

Thaddy de Koning

2014-08-17 04:26

reporter   ~0076620

Last edited: 2014-08-17 11:27

View 8 revisions

One way to test this maybe to create a console explicitly in a windows process. In that case I would expect output similar to linux, since that console would be owned by the windows process..
Change the apptype to {$APPTYPE GUI}, add windows to uses clause and then
begin
  AllocConsole;// add this to your code
  writeln('-->Start');
  Interf;
  writeln('-->End');
  readln;
  FreeConsole;// and this
end.

But:
This program crashed with an io error and outputs just Start/End.
Is that what you would expect ;)

Thaddy de Koning

2014-08-17 08:04

reporter   ~0076622

Last edited: 2014-08-17 11:24

View 10 revisions

So for windows there's no bug. for other OS's there may very well be one.

It may also be a documentation issue since the console under windows behaves very different from a linux console. E.G: Under windows there can be only one (1) console per process. This is allegedly not the case for Linux, which can have a (virtual) console per thread. And the console code in fpc is written in such a way that it expects multiple threads with a console attached to be possible, even under windows.
I have mentioned this before in the context of IO and this may be related.

I suspect is is possible that the linux output is caused by (console) thread termination before program termination. This is completely logical if the process owns the console instead of the other way around as under windows.

Marco van de Voort

2014-08-17 08:35

manager   ~0076623

Where is documented that it should be kept till the end of the function? IIRC it should be kept for the duration of the statement the last reference is in.

Sounds like relying on implementation defined behaviour.

Thaddy de Koning

2014-08-17 08:37

reporter   ~0076624

@Marco: crossed. Read my edit above. What do you think?

Thaddy de Koning

2014-08-17 08:40

reporter   ~0076625

I mean it is obvious that the OS implementaion of the console can play a role here.

Do-wan Kim

2014-08-17 11:43

reporter   ~0076626

if you want expected result, variable also defined like this.

var
  IMyUnk:IUnknown;

begin
  writeln('-->Start');
  IMyUnk:=Interf;
  writeln('-->End');
  readln;
end.

Thaddy de Koning

2014-08-17 12:08

reporter   ~0076627

Last edited: 2014-08-17 12:13

View 3 revisions

@Marco: You write "Where is documented that it should be kept till the end of the function? IIRC it should be kept for the duration of the statement the last reference is in." I think this is may be true for interfaces in general but AFAIK not for COM interfaces. In the case of COM the lifetime of an object is at the minimum determined by the codeblock it is in. That is needed because of the way COM marshaling works (extra-process) and because COM objects are never stack based and always on the heap. The marshaler kicks in only at the end of a determined codeblock to look for something to do.

stocki

2014-08-17 12:20

reporter   ~0076629

Last edited: 2014-08-17 12:41

View 4 revisions

The intf reference must stay alive until the procedure goes out of scope. That is the definition of an interface. (no need for "var IMyUnk:IUnknown")

Is there a difference with

IMyUnk:=Interf;

and

Interf;

???

stocki

2014-08-17 12:26

reporter   ~0076630

Last edited: 2014-08-17 12:33

View 4 revisions

just fyi:

1) The problem has nothing to do with console/gui/etc..

2) I wrote this "-->Destroy: You should not see this in console!" because the console window is being closed at that point.

Please replace

-->Destroy: You should not see this in console!

by

-->Destroy: You MUST NOT see this before -->End!

Thaddy de Koning

2014-08-17 13:13

reporter   ~0076631

Last edited: 2014-08-17 13:20

View 4 revisions

@Stocki In that case I can not reproduce it with current trunk under windows. As per my first comment. You state that you used trunk.
I can only replicate it under linux and with 2.6.X
And that may be caused by the way the OS's handles consoles. I did quite some research here in the night.
It doesn't mean it has something to do with the console perse, but you can not test this with a console except as per my little example. You are not looking at what you think you are looking at., at least under windows.
Can you think of an example w/o console that shows your perceived bug?

stocki

2014-08-17 13:17

reporter   ~0076632

I have used trunk 2.7.1 from yesterday. Reproduced under Win32. Do you really need a prog without console? I got the same bug. Interface support is broken in FPC.

stocki

2014-08-17 14:13

reporter   ~0076633

Last edited: 2014-08-17 14:16

View 3 revisions

A new program that shows the bug. The interface is broken when IUnkown result is not assigned to a stack variable.

FPC:
-->Start
-->Destroy: It is a bug, when you see this comment before ''-->End'' comment!
-->End

Delphi:
-->Start
-->End
-->Destroy: It is a bug, when you see this comment before ''-->End'' comment!


program project1;

{$APPTYPE console}

uses
  sysutils, classes;

type
  TIntfFoo = class(TInterfacedObject, IUnknown)
  public
    destructor Destroy; override;
  end;

destructor TIntfFoo.Destroy;
begin
  writeln('-->Destroy: It is a bug, when you see this comment before ''-->End'' comment!');
  inherited;
end;

function Interf: IUnknown;
begin
  Result := TIntfFoo.Create;
end;

procedure UseInterface_Bug;
begin
  writeln('-->Start');
  Interf;
  writeln('-->End');
  writeln('-->Proc goes of scope. After that Destroy must be called!');
end;

procedure UseInterface_ok;
var
  i: IUnknown;
begin
  writeln('-->Start');
  i := Interf;
  writeln('-->End');
  writeln('-->Proc goes of scope. After that Destroy must be called!');
end;


begin
  UseInterface_Bug;
  //UseInterface_Ok;
  readln;
end.

Max Nazhalov

2014-08-17 14:21

reporter   ~0076634

Last edited: 2014-08-17 15:32

View 2 revisions

@stocki:
"Is there a difference with
IMyUnk:=Interf;
and
Interf;"

In the second case You throw away the function result, and, I suspect, the temp variable holding it immediately goes out from the scope.
I wonder if the C++ behaves differently, leaking the instance.. Else it would be a kind of "automatic garbage collection", which is doubtfully practical in an unmanaged code.

Marco van de Voort

2014-08-17 15:58

manager   ~0076635

Last edited: 2014-08-17 16:04

View 2 revisions

1. I don't see a COM requirement to wait till the end of the function, please explain.
2. I do know that testing general automated types cleanup with the main program can be confusing. To avoid that, move the main program body to a procedure and call procedure that from the main program. File behaviour that only happens in the main program or with globals separately from a general case.

My comments were more meant for the general case.

stocki

2014-08-17 16:05

reporter   ~0076636

@Marco: Done. See post from 2014-08-17 14:13

Burkhard Carstens

2014-08-17 17:02

reporter   ~0076637

@stocki: as soon as reference count of the instance drops to zero, the instance is freed. If you don't assign the result of Interf to a variable, it has a refcount of zero after Interf returns unless the compiler decides to have another reference in a tmp var. In this case (tmp var), the instance is freed when the tmp var goes out of scope, either by assigning another instance to the tmp var or by reaching end of the function.
Till now, I've only seen complaints about instances being freed too late due to use of tmp var.

This is not a Bug but expected and well documented behaviour.

Jonas Maebe

2014-08-17 17:29

manager   ~0076638

@Stocki: your test program demonstrates the behaviour of the Delphi code generator, which can be different from what the COM standard itself guarantees.

http://msdn.microsoft.com/en-us/library/windows/desktop/ms687260(v=vs.85).aspx states "COM's method of determining when it is appropriate to deallocate an object is manual reference counting. Each object maintains a reference count that tracks how many clients are connected to it — that is, how many pointers exist to any of its interfaces in any client." Since the reference counting is manual, the COM standard itself cannot guarantee anything regarding liveness (other than that the object is live as long as the reference count is >= 1). And since you don't create a pointer to the COM object, its reference count is 0 ("no clients are connected to it") and it can be freed at any time.

There are other scenarios where FPC keeps a reference counted pointer around for a longer time than Delphi. See 0016578 , the related bug reports and the linked discussion on the mailing list.

Max Nazhalov

2014-08-17 17:31

reporter   ~0076639

@Burkhard Carstens
"If you don't assign the result of Interf to a variable, it has a refcount of zero"

Well, it does not seem quite accurate, but this does not matter. (See e.g. http://msdn.microsoft.com/en-us/library/windows/desktop/ms692481(v=vs.85).aspx
"Out parameters from functions, including return values.
To set the out parameter, the function must have a stable copy of the interface pointer. On return, the caller is responsible for releasing the pointer."

I.e. refcount is <>0 on return, so there is always some kind of tempvar involved (either in memory or in register).
In any way, lifetime of an abstract tempvar is an "implementation detail", and should not be relied upon.

stocki

2014-08-17 19:08

reporter   ~0076640

All said may be true, but the different behavior of FPC and Delphi in that case is really a problem for me.

Max Nazhalov

2014-08-17 19:18

reporter   ~0076641

Suggested workaround of using an explicit local variable will always work for You.
Even in Delphi.

Jonas Maebe

2014-08-17 19:56

manager   ~0076642

We don't emulate the code generator of Delphi, nor do we guarantee the same behaviour as Delphi when it's undocumented (it's too hard to get that right in all cases, and may even change from one Delphi version to another).

stocki

2014-08-23 17:07

reporter   ~0076684

Then you will get some nice Access Violations elsewhere and fpc will be unusable for Interface users.

Jonas Maebe

2014-08-23 17:28

manager   ~0076685

Your code is buggy and just happens to work by chance in Delphi. There is no guarantee anywhere in the Delphi or Microsoft documentation that an interface returned by a routine but not assigned to anything will remain live until the end of the caller. As long as you do not rely on undefined behaviour, interfaces work fine in both FPC and Delphi.

Issue History

Date Modified Username Field Change
2014-08-17 02:31 stocki New Issue
2014-08-17 03:09 Do-wan Kim Note Added: 0076618
2014-08-17 03:57 Thaddy de Koning Note Added: 0076619
2014-08-17 04:02 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:04 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:10 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:14 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:16 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:20 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:22 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 04:26 Thaddy de Koning Note Added: 0076620
2014-08-17 05:15 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 05:17 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 05:21 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 05:22 Do-wan Kim Note Edited: 0076618 View Revisions
2014-08-17 05:24 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 07:46 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 07:49 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 07:57 Thaddy de Koning Note Edited: 0076619 View Revisions
2014-08-17 08:04 Thaddy de Koning Note Added: 0076622
2014-08-17 08:05 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:15 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 08:17 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:22 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:29 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:31 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:32 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:32 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:35 Marco van de Voort Note Added: 0076623
2014-08-17 08:36 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 08:37 Thaddy de Koning Note Added: 0076624
2014-08-17 08:40 Thaddy de Koning Note Added: 0076625
2014-08-17 11:24 Thaddy de Koning Note Edited: 0076622 View Revisions
2014-08-17 11:27 Thaddy de Koning Note Edited: 0076620 View Revisions
2014-08-17 11:43 Do-wan Kim Note Added: 0076626
2014-08-17 12:08 Thaddy de Koning Note Added: 0076627
2014-08-17 12:09 Thaddy de Koning Note Edited: 0076627 View Revisions
2014-08-17 12:13 Thaddy de Koning Note Edited: 0076627 View Revisions
2014-08-17 12:20 stocki Note Added: 0076629
2014-08-17 12:26 stocki Note Added: 0076630
2014-08-17 12:31 stocki Note Edited: 0076630 View Revisions
2014-08-17 12:32 stocki Note Edited: 0076630 View Revisions
2014-08-17 12:33 stocki Note Edited: 0076630 View Revisions
2014-08-17 12:40 stocki Note Edited: 0076629 View Revisions
2014-08-17 12:40 stocki Note Edited: 0076629 View Revisions
2014-08-17 12:41 stocki Note Edited: 0076629 View Revisions
2014-08-17 13:13 Thaddy de Koning Note Added: 0076631
2014-08-17 13:14 Thaddy de Koning Note Edited: 0076631 View Revisions
2014-08-17 13:16 Thaddy de Koning Note Edited: 0076631 View Revisions
2014-08-17 13:17 stocki Note Added: 0076632
2014-08-17 13:20 Thaddy de Koning Note Edited: 0076631 View Revisions
2014-08-17 14:13 stocki Note Added: 0076633
2014-08-17 14:15 stocki Note Edited: 0076633 View Revisions
2014-08-17 14:16 stocki Note Edited: 0076633 View Revisions
2014-08-17 14:21 Max Nazhalov Note Added: 0076634
2014-08-17 15:32 Max Nazhalov Note Edited: 0076634 View Revisions
2014-08-17 15:58 Marco van de Voort Note Added: 0076635
2014-08-17 16:04 Marco van de Voort Note Edited: 0076635 View Revisions
2014-08-17 16:05 stocki Note Added: 0076636
2014-08-17 17:02 Burkhard Carstens Note Added: 0076637
2014-08-17 17:29 Jonas Maebe Note Added: 0076638
2014-08-17 17:31 Max Nazhalov Note Added: 0076639
2014-08-17 19:08 stocki Note Added: 0076640
2014-08-17 19:18 Max Nazhalov Note Added: 0076641
2014-08-17 19:56 Jonas Maebe Note Added: 0076642
2014-08-17 19:56 Jonas Maebe Status new => resolved
2014-08-17 19:56 Jonas Maebe Resolution open => no change required
2014-08-17 19:56 Jonas Maebe Assigned To => Jonas Maebe
2014-08-23 17:07 stocki Note Added: 0076684
2014-08-23 17:07 stocki Status resolved => feedback
2014-08-23 17:07 stocki Resolution no change required => reopened
2014-08-23 17:28 Jonas Maebe Note Added: 0076685
2014-08-23 17:28 Jonas Maebe Status feedback => resolved
2014-08-23 17:28 Jonas Maebe Resolution reopened => no change required
2018-10-04 16:10 Michael Van Canneyt Relationship added related to 0030409