Recent

Author Topic: setshape memory leak  (Read 5094 times)

djdjdjole

  • Full Member
  • ***
  • Posts: 101
setshape memory leak
« on: December 30, 2017, 10:45:10 am »
My application relies on frequent call to TWinControl.setShape method. After some time, application reports exception and call Stack points to that method. To be sure, I compiled Lazarus project (../examples/shapedcontrols), which I changed a little bit. Instead of a single call to setShape I inserted "for" loop to call it 100000 times. After some time I received the same error as in my original application. Specifically- Lazarus debugger breaks at BitmapToRegion method in win32proc unit, after calling GlobalAlloc (which expresses not enough memory).
Windows10, Lazarus 1.6.

Thanks

Thaddy

  • Hero Member
  • *****
  • Posts: 10436
Re: setshape memory leak
« Reply #1 on: December 30, 2017, 01:53:26 pm »
Simple and compilable example code plz? We will fix such things in minutes.

But let me guess: your "changed a little bit.." uses the same variable....10.000 times?, then you are creating 9999 leaks!...Because you did not free them in between. The alternative is to use 10.000 different instances and properly free them.
« Last Edit: December 30, 2017, 01:58:58 pm by Thaddy »
When you ask a question that is actually answered in the documentation, you are either lazy or a moron.

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #2 on: December 30, 2017, 04:18:30 pm »
I am thankful to my friend, who found a bug in BitmapToRegion function. What should be added to the end of it is :
GlobalUnlock(hData);
GlobalFree(hData);

Thanks

avra

  • Hero Member
  • *****
  • Posts: 1998
    • Additional info
Re: setshape memory leak
« Reply #3 on: January 01, 2018, 11:44:52 am »
We are still waiting for a simple compilable example that could help us to determine if that is really a memory leak that should be fixed, or misuse on your side.
ct2laz - Conversion between Lazarus and CodeTyphon
bithelpers - Bit manipulation for standard types
pasettimino - Siemens S7 PLC lib

Thaddy

  • Hero Member
  • *****
  • Posts: 10436
Re: setshape memory leak
« Reply #4 on: January 01, 2018, 12:32:22 pm »
I am thankful to my friend, who found a bug in BitmapToRegion function. What should be added to the end of it is :
GlobalUnlock(hData);
GlobalFree(hData);

Thanks

That can only be the case if you added a GlobalLock yourself... which is YOUR bug....Unless you can prove that the lock is called at least twice and te release is only once. Which I can't....I can only disprove the issue under normal behavior.
When you ask a question that is actually answered in the documentation, you are either lazy or a moron.

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #5 on: January 01, 2018, 12:39:00 pm »
Excuse me- you are right. I can hardly present complete source code from my application, although TPump.Paint is (TPump.Invalidate is called 50 times for different instances of TPump, twice in second):

procedure TPump.Paint;  //I know that "Self" here is redundant, but it stands in my source code
var
  Bitmap: TBitmap;
begin
  Bitmap := MyShape;
  try
    Self.SetShape(Bitmap);  // this is where debugger breaks after 20 minutes approx. (after GlobalAlloc I wrote about)
  finally
    Bitmap.Free;
  end;
  Bitmap := MyBitmap;
  try
    Self.Canvas.Draw(0,0,Bitmap);
  finally
    Bitmap.Free;
  end;
  inherited Paint;
end;

Of course, I know that for complete example, I should send MyShape and MyBitmap implementation and details about TPump class (which inherits from TCustomControl),  but I don't know how to submit everything needed, yet to be enough for this kind of correspondence.

Regarding Lazarus example, I wrote about and that I changed "a little bit", it is:

procedure TForm1.Button1Click(Sender: TObject);
begin
  ShapeControl(Self);
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  Button1.Handle;
  ShapeControl(Button1);
end;

procedure TForm1.ShapeControl(AControl: TWinControl);
var
  ABitmap: TBitmap;
  Points: array of TPoint;      i: integer;  // I added variable "i"
begin
  ABitmap := TBitmap.Create;
  ABitmap.Monochrome := True;
  ABitmap.Width := AControl.Width;
  ABitmap.Height := AControl.Height;
  SetLength(Points, 6);
  Points[0] := Point(0, ABitmap.Height div 2);
  Points[1] := Point(10, 0);
  Points[2] := Point(ABitmap.Width - 10, 0);
  Points[3] := Point(ABitmap.Width, ABitmap.Height div 2);
  Points[4] := Point(ABitmap.Width - 10, ABitmap.Height);
  Points[5] := Point(10, ABitmap.Height);
  with ABitmap.Canvas do
  begin
    Brush.Color := clBlack; // transparent color
    FillRect(0, 0, ABitmap.Width, ABitmap.Height);
    Brush.Color := clWhite; // mask color
    Polygon(Points);
  end;
// next line- I added "for" loop - originally only AControl.SetShape exists in example
  for i:= 1 to 100000 do AControl.SetShape(ABitmap);  // debugger breaks after minute approx.
  ABitmap.Free;
end;

Regards

Thaddy

  • Hero Member
  • *****
  • Posts: 10436
Re: setshape memory leak
« Reply #6 on: January 01, 2018, 01:18:56 pm »
That is exactly what I predicted before I actually saw your source!
Quote from: thaddy
But let me guess: your "changed a little bit.." uses the same variable....10.000 times?, then you are creating 9999 leaks!...Because you did not free them in between. The alternative is to use 10.000 different instances and properly free them.
Code: Pascal  [Select][+][-]
  1. { THIS IS COMPLETELY WRONG!! }
  2. // next line- I added "for" loop - originally only AControl.SetShape exists in example
  3.   for i:= 1 to 100000 do AControl.SetShape(ABitmap);  // debugger breaks after minute approx.
  4.   ABitmap.Free;
  5. end;
« Last Edit: January 01, 2018, 01:25:15 pm by Thaddy »
When you ask a question that is actually answered in the documentation, you are either lazy or a moron.

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #7 on: January 01, 2018, 01:29:38 pm »
Well, it seemed to me that it was ok to use ABitmap many times, after it was created (beginning of proc) and then free it (last line). Obviously my mistake.

Regards

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #8 on: January 01, 2018, 02:09:11 pm »
Although it is probably Object programming oriented question, I am interested if this would be OK regarding memory issue:

procedure TForm1.ShapeControl(AControl: TWinControl);
var
  ABitmap: TBitmap;
  Points: array of TPoint;      i: integer;
begin
  for i:= 1 to 100000 do  // for loop inserted here
  begin

  ABitmap := TBitmap.Create;
  ABitmap.Monochrome := True;
  ABitmap.Width := AControl.Width;
  ABitmap.Height := AControl.Height;
  SetLength(Points, 6);
  Points[0] := Point(0, ABitmap.Height div 2);
  Points[1] := Point(10, 0);
  Points[2] := Point(ABitmap.Width - 10, 0);
  Points[3] := Point(ABitmap.Width, ABitmap.Height div 2);
  Points[4] := Point(ABitmap.Width - 10, ABitmap.Height);
  Points[5] := Point(10, ABitmap.Height);
  with ABitmap.Canvas do
  begin
    Brush.Color := clBlack; // transparent color
    FillRect(0, 0, ABitmap.Width, ABitmap.Height);
    Brush.Color := clWhite; // mask color
    Polygon(Points);
  end;
  AControl.SetShape(ABitmap);
  ABitmap.Free;
  end;
end;

Regards

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #9 on: January 01, 2018, 04:09:56 pm »
Well, let me try to clarify. The example I rewrote in my last reply could still be simplified  like:

procedure TForm1.ShapeControl(AControl: TWinControl);
var
  ABitmap: TBitmap;
     i: integer;
begin
  for i:= 1 to 100000 do  // for loop inserted here
  begin
  ABitmap := TBitmap.Create;
  ABitmap.Monochrome := True;
  ABitmap.Width := AControl.Width;
  ABitmap.Height := AControl.Height;
  with ABitmap.Canvas do
  begin
    Brush.Color := clBlack; // transparent color
    FillRect(0, 0, ABitmap.Width, ABitmap.Height);
    Brush.Color := clWhite; // mask color
  end;
  AControl.SetShape(ABitmap);
  ABitmap.Free;
  end;
end;

Run this example and you will still have memory leak at SetShape method after a minute.
ABitmap is now created and freed, the way Thaddy suggested (if I understood him right).

Regards
 

GetMem

  • Hero Member
  • *****
  • Posts: 3757
Re: setshape memory leak
« Reply #10 on: January 01, 2018, 06:00:25 pm »
The problem is that GlobalAlloc api at some point fails, the author of the function "BitmapToRegion" did not check the return value of GlobalAlloc, if it's zero, the function should exit immediately, but not before freeing other data. The correct implementation should look like this:
Code: Pascal  [Select][+][-]
  1. function BitmapToRegion(hBmp: HBITMAP; cTransparentColor: COLORREF = 0; cTolerance: COLORREF  = $101010): HRGN;
  2. begin
  3.   //...
  4.   hData := GlobalAlloc(GMEM_MOVEABLE, sizeof(RGNDATAHEADER) + (sizeof(TRECT) * maxRects));
  5.   if hData = 0 then
  6.   begin
  7.     Freemem(Data);
  8.     Exit;
  9.   end;
  10.   //...                
  11. end;
Make the necessary modifications, rebuild the IDE, after your program no longer leaks.
Other then this, creating 100000 bitmaps, then set the controls shape 100000 times makes no sense at all. What exactly are you trying to achieve?

djdjdjole

  • Full Member
  • ***
  • Posts: 101
Re: setshape memory leak
« Reply #11 on: January 01, 2018, 07:11:45 pm »
It wasn't my point to create 100000 bitmaps. It was something that helped me explain (to forum people) that something was wrong with the method. My real application just makes frequent calls to setShape and after some time (it is in fact few days of on field working) I received end user feedback that it "stops working". After many attempts, I finally was able to put the finger on setShape.
Anyway thanks for explanation.

Regards

GetMem

  • Hero Member
  • *****
  • Posts: 3757
Re: setshape memory leak
« Reply #12 on: January 01, 2018, 07:23:24 pm »
@djdjdjole
Quote
It wasn't my point to create 100000 bitmaps. It was something that helped me explain (to forum people) that something was wrong with the method
OK. Fair enough.  :)

Quote
I received end user feedback that it "stops working". After many attempts, I finally was able to put the finger on setShape. Anyway thanks for explanation.
If you modify BitmapToRegion function, your application won't leak anyomore(just tested).

 

TinyPortal © 2005-2018