View Issue Details

IDProjectCategoryView StatusLast Update
0024099FPCRTLpublic2017-07-28 01:08
ReporteroceanAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.7.1Product Build 
Target Version3.2.0Fixed in Version3.1.1 
Summary0024099: Crash on "free" when using heaptrc
DescriptionAttached (Windows only) project crashes, when you click button.

It only crashes, if heaptrc option is checked.

shdocvw_1_1_tlb is from import typelibrary ieframe.dll

Similar issue http://lists.freepascal.org/lists/fpc-devel/2012-November/030356.html

Did not find responses to that.
TagsEventSink, TEventSink
Fixed in Revision36771
FPCOldBugId
FPCTarget
Attached Files
  • crash.zip (17,341 bytes)
  • eventsink_selfdestroy.pp.patch (566 bytes)
    Index: eventsink.pp
    ===================================================================
    --- eventsink.pp	(revision 23683)
    +++ eventsink.pp	(working copy)
    @@ -198,9 +198,12 @@
     begin
      if Assigned(FDispatch) then begin
       // Unhook the sink from the automation server
    +  Self._AddRef; (*Ensures a copy of self is alive while disconnecting
    +                  because Interface Disconnect will call release after disconnect.*)
       InterfaceDisconnect(FDispatch, FDispIntfIID, FConnection);
       FDispatch := nil;
       FConnection := 0;
    +  Self._Release;
      end;
     end;
     
    

Activities

ocean

2013-03-21 13:10

reporter  

crash.zip (17,341 bytes)

Jonas Maebe

2013-03-21 18:25

manager   ~0066473

It is quite likely that this is a program error rather than a compiler error. The fact that it seems to work without errors without heaptrc does not prove anything. In fact, one of the functionalities of heaptrc is that it will make crashes when using e.g. previously freed memory blocks much more likely.

José Mejuto

2013-03-21 22:00

reporter  

eventsink_selfdestroy.pp.patch (566 bytes)
Index: eventsink.pp
===================================================================
--- eventsink.pp	(revision 23683)
+++ eventsink.pp	(working copy)
@@ -198,9 +198,12 @@
 begin
  if Assigned(FDispatch) then begin
   // Unhook the sink from the automation server
+  Self._AddRef; (*Ensures a copy of self is alive while disconnecting
+                  because Interface Disconnect will call release after disconnect.*)
   InterfaceDisconnect(FDispatch, FDispIntfIID, FConnection);
   FDispatch := nil;
   FConnection := 0;
+  Self._Release;
  end;
 end;
 

José Mejuto

2013-03-21 22:00

reporter   ~0066478

Hello,

Apply the attached patch to packages\winunits-base\src\eventsink.pp

ocean

2013-03-22 16:21

reporter   ~0066494

Patch works, thanks

Reinier Olislagers

2014-09-06 13:34

developer   ~0076929

Seems Ludo Brands hasn't been responding for a while - perhaps somebody else (Marco?) can commit this if ok?

Marco van de Voort

2014-09-06 15:54

manager   ~0076936

I think I looked at it before, and wondered if it didn't need a try..finally, and decided to wait for Ludo.

Any opinions to that?

José Mejuto

2014-09-06 21:01

reporter   ~0076944

Hello,

If an exception raises inside InterfaceDisconnect the dangling reference is not important. If you add a try .. finally the situation would be the same as before because or InterfaceDisconnect is properly disconnected of next event to be "sinked" will find an already free object and crash! which IMHO is worst as you will get apparently unrelated exceptions.

The bug was quite difficult to spot as one do not expect a function to release itself and continue executing... The culprits are:

   FDispatch := nil;
   FConnection := 0;

which are local. Maybe not setting them it will be fixed same way :-?

Michael Van Canneyt

2017-07-23 12:23

administrator   ~0101868

Applied patch after testing it actually solves the issue.

Issue History

Date Modified Username Field Change
2013-03-21 13:10 ocean New Issue
2013-03-21 13:10 ocean File Added: crash.zip
2013-03-21 18:25 Jonas Maebe Note Added: 0066473
2013-03-21 22:00 José Mejuto File Added: eventsink_selfdestroy.pp.patch
2013-03-21 22:00 José Mejuto Note Added: 0066478
2013-03-22 16:21 ocean Note Added: 0066494
2013-04-29 10:41 Denis Kozlov Tag Attached: EventSink
2013-04-29 10:41 Denis Kozlov Tag Attached: TEventSink
2013-05-04 19:05 Marco van de Voort Assigned To => Ludo Brands
2013-05-04 19:05 Marco van de Voort Status new => assigned
2014-09-06 13:34 Reinier Olislagers Note Added: 0076929
2014-09-06 15:54 Marco van de Voort Note Added: 0076936
2014-09-06 21:01 José Mejuto Note Added: 0076944
2017-07-23 12:23 Michael Van Canneyt Fixed in Revision => 36771
2017-07-23 12:23 Michael Van Canneyt Note Added: 0101868
2017-07-23 12:23 Michael Van Canneyt Status assigned => resolved
2017-07-23 12:23 Michael Van Canneyt Fixed in Version => 3.1.1
2017-07-23 12:23 Michael Van Canneyt Resolution open => fixed
2017-07-23 12:23 Michael Van Canneyt Assigned To Ludo Brands => Michael Van Canneyt
2017-07-23 12:23 Michael Van Canneyt Target Version => 3.2.0