Can this cause AV?

[Fibonacci]

I have a pretty big app which crashes sometimes, Im looking where is the bug and cant find it.

So I wonder if this might be the cause.

I have a class which has a method which frees the object (itself) and returns something.

Something like this:

Code: Pascal  [Select][+][-]

1. type
2.   tclass = class
3.     somedata: string;
4.     function add(s: string): tclass;
5.     function finished: string;
6.   end;
7.  
8. function tclass.add(s: string): tclass;
9. begin
10.   somedata += s;
11.   result := self;
12. end;
13.  
14. function tclass.finished: string;
15. begin
16.   result := somedata;
17.   self.Free;
18. end;
19.  
20. function things: tclass;
21. begin
22.   result := tclass.Create;
23. end;
24.  
25. var
26.   s: string;
27.  
28. begin
29.   s := things.add('this').add('that').finished;
30.   writeln(s);
31.   readln;
32. end.

Can this cause Access Violations?

[Martin_fr]

In should be fine. The concept used in finish is ok.

Can you reproduce the crash at will, or does it happen very often (so you could do a few runs, and be sure it would happen)

1) Have you tried compiling without  optimization?

2) heaptrc -gh with env set HEAPTRC=keepreleased

3) If on Linux: run under valgrind --tool=memcheck

[Fibonacci]

Thanks for confirming thats not it.

The code is pretty big, crashes happen on random, on different machines (not on my dev machine unfortunately, also not on virtual machines), hard to debug. Heaptrc tells something is freed TWICE (was released at -> freed again at), unfortunately I cant see which line is that, stack trace is just addresses. Optimizations on or off makes no difference.

In other apps stack trace contains some info about where it is, but not in this particular app. Just addresses, not much.

But yes, I can force it to crash by forcing it to do much work, so I keep digging.

[Martin_fr]

You should try valgrind. If heaptrc can detect it, valgrind may find more. Valgrind keeps track of every alloc and free. And if any of them goes wrong (double), even if there is no crash, it will tell you. And it tells you the location of the alloc and the free.

For best results, you may need to recompile packages with debug info, if they aren't already.

Only drawback:
- valgrind is Linux only
- get a big coffee, it's gonna take a while. Under valgrind your app runs at a tiny fraction of its normal speed.

[marcov]

Tclass is a predefined type, so maybe that goes wrong here and there?

[Fibonacci]

Tclass is a predefined type, so maybe that goes wrong here and there?

Nah, the class has a different name I named it tclass just for this example code

[Thaddy]

Yes that can easily lead to stale pointers, so after self.free set self to nil.
Or call FreeAndNil(self). In this case I will allow it.

BTW TClass is a built-in so as Marco wrote it can get you into trouble.
TClass is a reference to TObject. Not a class.

[BrunoK]

But yes, I can force it to crash by forcing it to do much work, so I keep digging.

In debug mode, and with a bit of luck, adding a free counter to the class and code in the .finished method might help you figure out from where an eventual duplicate free is called.

Code: Pascal  [Select][+][-]

1. uses
2.   sysutils;
3.  
4. type
5.   TTestClass = class
6.     dbgDblFree: integer;
7.  
8.     somedata: string;
9.     function add(s: string): TTestClass;
10.     function finished: string;
11.   end;
12.  
13. function TTestClass.add(s: string): TTestClass;
14. begin
15.   somedata += s;
16.   result := self;
17. end;
18.  
19. function TTestClass.finished: string;
20. begin
21.   result := somedata;
22.   if dbgDblFree < 0 then
23.     Raise Exception.Create('Double free'); // <- break point here
24.   Dec(dbgDblFree);  // Double free detection
25.   self.Free;
26. end;
27.  
28. function things: TTestClass;
29. begin
30.   result := TTestClass.Create;
31. end;
32.  
33. var
34.   s: string;
35.  
36. begin
37.   s := things.add('this').add('that').finished;
38.   writeln(s);
39.   readln;
40. end.

More complicated would be to link with a debug version of FPC.

[Martin_fr]

Yes that can easily lead to stale pointers, so after self.free set self to nil.

Why? "Self := nil" would not set the callers variable to nil. And the local variable "Self" itself is definitely not used after that (part of the concept is, that it freeing it is the last thing in the method.

In debug mode, and with a bit of luck, adding a free counter to the class and code in the .finished method might help you figure out from where an eventual duplicate free is called.

As long as he calls free after the "dec(dbgDblFree)" the field itself becomes trashed, and its value no longer exists. A 2nd call would not see it.

However, a test run without "free" (leaking all the objects) would make the counter work.

[Fibonacci]

Or call FreeAndNil(self). In this case I will allow it.

Thanks Thaddy!

BTW TClass is a built-in so as Marco wrote it can get you into trouble.
TClass is a reference to TObject. Not a class.

I already said its just for the sample code. I wont use it for sample codes anymore though


@BrunoK: Thanks for the suggestion, but there is absolutely no chance to double call ".finished". I think the problem is much deeper. Its multithreaded app with many critical sections, at some point memory gets corrupted, sometimes earlier, sometimes a little later.

[Thaddy]

Why? "Self := nil" would not set the callers variable to nil. And the local variable "Self" itself is definitely not used after that (part of the concept is, that it freeing it is the last thing in the method.

No. Look at the implementation of freeandnil. That is for a reason.

[Fibonacci]

No. Look at the implementation of freeandnil. That is for a reason.

"Self" is temporary local pointer to the class, what is the point of setting it to nil if its never used anymore? Isnt it like setting local Integer "i := 0" at the end of a function?

[Martin_fr]

@BrunoK: Thanks for the suggestion, but there is absolutely no chance to double call ".finished". I think the problem is much deeper. Its multithreaded app with many critical sections, at some point memory gets corrupted, sometimes earlier, sometimes a little later.

valgrind also has 2 thread checkers (DRD, Helgrind). They do report a lot of false positives. But as a last resort...

[Fibonacci]

Its Windows only app

[BrunoK]

EDIT : ignore this message, it is wrong !
EDIT : ignore this message, it is wrong !
EDIT : ignore this message, it is wrong !

In debug mode, and with a bit of luck, adding a free counter to the class and code in the .finished method might help you figure out from where an eventual duplicate free is called.

As long as he calls free after the "dec(dbgDblFree)" the field itself becomes trashed, and its value no longer exists. A 2nd call would not see it.

dbgDblFree trashed : not so much.

Slightly modified test code to show that :

Code: Pascal  [Select][+][-]

1. program p;
2. uses
3.   SysUtils;
4. type
5.  
6.   { TTestClass }
7.  
8.   TTestClass = class
9.     somedata: string;
10.     dbgDblFree: integer;
11.     function add(s: string): TTestClass;
12.     function finished: string;
13.     destructor Destroy; override;
14.   end;
15.  
16. var
17.   vTestInst: TTestClass;
18.   s1: string;
19.  
20.   function TTestClass.add(s: string): TTestClass;
21.   begin
22.     somedata += s;
23.     Result := self;
24.   end;
25.  
26.   function TTestClass.finished: string;
27.   begin
28.     s1 := HexStr(IntPtr(Self), SizeOf(IntPtr));
29.     Result := somedata;
30.     self.Free;
31.   end;
32.  
33.   destructor TTestClass.Destroy;
34.   begin
35.     Dec(dbgDblFree);  // Double free detection
36.     if dbgDblFree < 0 then
37.       raise Exception.Create('Double free'); // <- break point here
38.     inherited Destroy;
39.   end;
40.  
41.   function things: TTestClass;
42.   begin
43.     Result := TTestClass.Create;
44.     vTestInst := Result; // Store it to do a double free ~bk
45.   end;
46.  
47. var
48.   s: string;
49. begin
50.   s := things.add('this').add('that').finished;
51.   writeln(s);
52.   s := vTestInst.finished;
53.   writeln(s);
54.   readln;
55. end.

win10, FPC 3.2.2