Jump to content
limelect

I solved my problem but

Recommended Posts

I solved my problem but i want to understand

Using this function

function GetUpDate(Link: string): string;
var
  S: string;
  IdHTTP: TIdHTTP;
begin
  IdHTTP := TIdHTTP.Create(nil);//Form1);
  try
    S := IdHTTP.Get(Link);
    result := S;
  except
    ShowMessage('Could not get VERSION information');
     end;

IdHTTP.Free;
end;

using IdHTTP := TIdHTTP.Create(nil) <<<< give me great amount of LEAKS,

using  IdHTTP := TIdHTTP.Create(Form1); <<< it is OK

WHY thanks

Edited by limelect

Share this post


Link to post

Hi...:classic_cool:

 

...always try/finally! :classic_tongue: In case of error IdHTTP.free is not executed because skipped to the end

 

Better:

 IdHTTP := TIdHTTP.Create(nil);
 try
   try
     S := IdHTTP.Get(Link);
     result := S;
   except
     ShowMessage('Could not get VERSION information');
   end;
finally
  IdHTTP.Free;
end;

 

Edited by haentschman

Share this post


Link to post

@haentschman I thought that too but i double checked

It does free http at list on the single step

I made a link error for that

Edited by limelect

Share this post


Link to post
51 minutes ago, limelect said:

@haentschman I thought that too but i double checked

It does free http at list on the single step

Did you actually test with identical except block? Because without try finally IdHTTP is not freed, if function is exited before free line. That is why using try finally is strongly recommended.

 

Share this post


Link to post
Just now, limelect said:

Thank but that is not MY QUESTION

Then what is the question?

In your original sample neither version leaks memory, because except block eats exception and free is called anyway.

 

if you put exit somewhere before IdHTTP.Free, then both versions initially leak memory, but in .Create(Form1) version it will be finally freed, when form is freed. But it is still type of memory leak, because every time code is executed it creates new object, but it is only freed, when form is freed. So memory usage will continue go up on every execution of GetUpDate until Form1 is freed.

Share this post


Link to post

Your code doesn't call Free if an exception is raised, when you pass nil as the owner. When you pass the form as an owner, the form destroys the owned object when the form itself is destroyed.

 

You need to learn how to use finally blocks.  This is a fundamental pattern that is essential knowledge.

Share this post


Link to post

 

Just now, David Heffernan said:

Your code doesn't call Free if an exception is raised

Are you sure about that? The code shown looks like this if formatted properly:

function GetUpDate(Link: string): string;
var
  S: string;
  IdHTTP: TIdHTTP;
begin
  IdHTTP := TIdHTTP.Create(nil);//Form1);
  try
    S := IdHTTP.Get(Link);
    result := S;
  except
    ShowMessage('Could not get VERSION information');
  end;
  IdHTTP.Free;
end;

The exception is handled with a ShowMessage call and after that the code following the end is executed, which calls IdHTTP.Free. Although I also prefer the try-finally approach, this code is perfectly valid, as long as ShowMessage returns as expected.

Share this post


Link to post

Sorry to be misunderstood

But in the above this is my question

using IdHTTP := TIdHTTP.Create(nil) <<<< give me great amount of LEAKS,

using  IdHTTP := TIdHTTP.Create(Form1); <<< it is OK

Ver simple question

Share this post


Link to post
8 minutes ago, limelect said:

using IdHTTP := TIdHTTP.Create(nil) <<<< give me great amount of LEAKS,

using  IdHTTP := TIdHTTP.Create(Form1); <<< it is OK

Ver simple question

The answer should also be very simple.

Make sure to do IdHTTP.Free; within a finally block.

 

Did you try that already??? And did you still have leaks ???

If you did and you still have leaks, the must come from somewhere else.

 

Edited by rvk

Share this post


Link to post
34 minutes ago, Uwe Raabe said:

 

Are you sure about that? The code shown looks like this if formatted properly:


function GetUpDate(Link: string): string;
var
  S: string;
  IdHTTP: TIdHTTP;
begin
  IdHTTP := TIdHTTP.Create(nil);//Form1);
  try
    S := IdHTTP.Get(Link);
    result := S;
  except
    ShowMessage('Could not get VERSION information');
  end;
  IdHTTP.Free;
end;

The exception is handled with a ShowMessage call and after that the code following the end is executed, which calls IdHTTP.Free. Although I also prefer the try-finally approach, this code is perfectly valid, as long as ShowMessage returns as expected.

You are right, I misread the code.

Share this post


Link to post
13 minutes ago, limelect said:

Sorry to be misunderstood

But in the above this is my question

using IdHTTP := TIdHTTP.Create(nil) <<<< give me great amount of LEAKS,

using  IdHTTP := TIdHTTP.Create(Form1); <<< it is OK

Ver simple question

Can you provide a minimal example that demonstrates the issue.

  • Like 1

Share this post


Link to post
3 hours ago, Uwe Raabe said:

 


function GetUpDate(Link: string): string;
var
  S: string;
  IdHTTP: TIdHTTP;
begin
  IdHTTP := TIdHTTP.Create(nil);//Form1);
  try
    S := IdHTTP.Get(Link);
    result := S;
  except
    ShowMessage('Could not get VERSION information');
  end;
  IdHTTP.Free;
end;

The exception is handled with a ShowMessage call and after that the code following the end is executed, which calls IdHTTP.Free. Although I also prefer the try-finally approach, this code is perfectly valid, as long as ShowMessage returns as expected.

 

This code is not 100% correct, though. If constructor fails, Free will be called on an uninitialized reference. In order for it to work properly, IdHTTP should be initialized to nil before calling constructor.

 

 

begin
  IdHTTP := nil;
  IdHTTP := TIdHTTP.Create(nil);
  ...

 

Edit... forget about that.... brain fart... if the exception is raised IdHTTP.Free will never be called.

 

Edited by Dalija Prasnikar

Share this post


Link to post
10 minutes ago, Dalija Prasnikar said:

 

This code is not 100% correct, though. If constructor fails, Free will be called on an uninitialized reference. In order for it to work properly, IdHTTP should be initialized to nil before calling constructor. 

No.... If constructor fails, then Free is never called.... Create is not inside try except block...

  • Like 1

Share this post


Link to post
1 hour ago, Dalija Prasnikar said:

This code is not 100% correct, though. If constructor fails, Free will be called on an uninitialized reference. In order for it to work properly, IdHTTP should be initialized to nil before calling constructor.

No it won't. If the constructor fails then an exception is raised before the try / except is active.

Share this post


Link to post
55 minutes ago, limelect said:

This is the minimal. Only that I have added FastMM4 to DPR

No memory leaks when I run your code. Can you provide a minimal but complete example that we can compile directly.

Share this post


Link to post
27 minutes ago, David Heffernan said:

No it won't. If the constructor fails then an exception is raised before the try / except is active.

Right... my brain is fried...

Share this post


Link to post
8 hours ago, limelect said:

using IdHTTP := TIdHTTP.Create(nil) <<<< give me great amount of LEAKS,

using  IdHTTP := TIdHTTP.Create(Form1); <<< it is OK

WHY thanks

You did not explain what kind of leaks you are actually experiencing (can you provide the actual leak report?), but I suspect the leaks have nothing to do with the TIdHTTP object itself.  Are you referring to the intentional leaks in Indy's IdStack.pas and IdThread.pas units?  If so, that is a completely separate matter, and one that is not affected by the Owner you assign to the TIdHTTP object.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×