0

my app have a problem. Longer run time = more MB in RAM. It start after i tryed to using Threads. Im using 3x Tread with timer. Threads im using because im using httpget, and without it GUI freeze always when it start call httpget that download file about 150 byte. Timer interval can be set at 5 - 300 seconds most of times is used 5s. So every timer time im calling 3x Thread and in each Thread im calling httpget. But app eating memory maybe im doing something wrong with threads? Maybe they stay "open" somehow?

var
  a, b, c: string;

type
  TThread_1 = class(TThread)
    private
      FVar_1: string;

      procedure Update_1;
    protected
      procedure Execute; override;
    end;

  TThread_2 = class(TThread)
    private
      FVar_2: string;

      procedure Update_2;
    protected
      procedure Execute; override;
    end;

  TThread_3 = class(TThread)
    private
      FVar_3: string;

      procedure Update_3;
    protected
      procedure Execute; override;
    end;


procedure TThread_1.Execute;
begin
  FVar_1 := HttpGet('url', 'a');
  Synchronize(Update_1);
end;

procedure TThread_1.Update_1;
begin
  a := FVar_1;
end;

procedure TThread_2.Execute;
begin
  FVar_2 := HttpGet('url', 'b');
  Synchronize(Update_2);
end;

procedure TThread_2.Update_2;
begin
  b := FVar_2;
end;

procedure TThread_3.Execute;
begin
  FVar_3 := HttpGet('url', 'c');
  Synchronize(Update_3);
end;

procedure TThread_3.Update_3;
begin
  c := FVar_3;
end;


procedure TForm1.Timer_thread(Sender: TObject);
var
  t1 : TThread_1;
  t2 : TThread_2;
  t3 : TThread_3;
begin
  if lb_1.IsChecked = True then
    begin
      t1 := TThread_1.Create(true);
      try
        t1.FreeOnTerminate := true;
      finally
        t1.Start;
      end;
    end;

  if lb_2.IsChecked = True then
    begin
      t2 := TThread_2.Create(true);
      try
        t2.FreeOnTerminate := true;
      finally
        t2.Start;
      end;
    end;

  if lb_3.IsChecked = True then
    begin
      t3 := TThread_3.Create(true);
      try
        t3.FreeOnTerminate := true;
      finally
        t3.Start;
      end;
    end;
end;

function HttpGet(const url: string; const tip: string): string;
var
  HTTP: TIdHTTp;
begin
  HTTP := TIdHTTP.Create(nil);
  HTTP.Request.UserAgent := 'Mozilla/6.0 (compatible; Delphi)';
  try
    try
      Result:=http.Get(url);
    except
      on E : Exception do
        begin
          if Pos('Error resolving Address', E.Message) <> 0 then
            begin
              if tip = '1' then Form1.lb_1.Text:='1';
              if tip = '2' then Form1.lb_2.Text:='2';
              if tip = '3' then Form1.lb_3.Text:='2';
            end;
        end;
    end;
  finally
    FreeAndNil (http);
  end;
end;

Edited code but still eating memory (1MB / 1 minute - timer interval 5 seconds), if timer turn off it stop eat memory:

procedure TForm1.Timer_thread(Sender: TObject);
var
  t1 : TThread;
  t2 : TThread;
  t3 : TThread;

begin
  if lb_1.IsChecked = True then
    begin
      t1:= TThread.CreateAnonymousThread(
        procedure
        var s: string;
          begin
            s := HttpGet('url_a', 'a');
            TThread.Synchronize(nil,
            procedure
              begin
                a := s;
              end);
          end);
      t1.Start;
    end;

  if lb_2.IsChecked = True then
    begin
      t2:= TThread.CreateAnonymousThread(
        procedure
        var s: string;
          begin
            s := HttpGet('url_b', 'b');
            TThread.Synchronize(nil,
            procedure
              begin
                b := s;
              end);
          end);
      t2.Start;
    end;

  if lb_3.IsChecked = True then
    begin
      t3:= TThread.CreateAnonymousThread(
        procedure
        var s: string;
          begin
            s := HttpGet('c', 'c');
            TThread.Synchronize(nil,
            procedure
              begin
                c := s;
              end);
          end);
      t3.Start;
    end;
end;
BOB
  • 23
  • 7
  • Close your threads: https://stackoverflow.com/questions/4044855/how-to-kill-a-thread-in-delphi#4044957 – Neel Kamath Jun 21 '18 at 03:49
  • 1
    @NeelKamath that is completely wrong and unrelated, and unnecessary anyway due to `FreeOnTerminate` being used. Something else is going on. – Remy Lebeau Jun 21 '18 at 04:38
  • 2
    What version of Delphi is being used? `Synchronize()` was broken on Android until XE7. And what does `HttpGet()` look like exactly? – Remy Lebeau Jun 21 '18 at 04:39
  • 3
    The use of try finally in the timer method is wrong. That isn't your problem but you should remove those three try finally blocks. If an exception is raised when setting FreeOnTerminate then you are safe to bail out then. It's wrong to start the thread in that eventuality. – David Heffernan Jun 21 '18 at 06:43
  • Remy: used Delphi 10.2, http will add to post David: so i will keep just try without finaly? and that start will move before try? – BOB Jun 21 '18 at 17:01
  • @DavidHeffernan setting `FreeOnTerminate` on a suspended thread will not raise an exception. Personally, I would override the thread constructors to set `FreeOnTerminate` and then create the threads without suspension, thus no need to call `Start` manually – Remy Lebeau Jun 21 '18 at 17:02
  • @RemyLebeau: thank you, but i dont have much experience, and threads im learning from source codes from here, and trying to understand it – BOB Jun 21 '18 at 17:29
  • @Remy That's tangential to my point about the try finally and the poor asker is now going to wonder whether or not the try finally is correct. Which we both know it isn't. – David Heffernan Jun 21 '18 at 18:32
  • @BOB Rather than using `FreeOnTerminate` at all, consider using `TThread.CreateAnonymousThread()` instead, eg: `TThread.CreateAnonymousThread(procedure var s: string; begin s := HttpGet('url', 'a'); TThread.Synchronize(nil, procedure begin a := s; end); end).Start;` – Remy Lebeau Jun 21 '18 at 19:05
  • @BOB Note, in `HttpGet()`, if an exception is raised (and comparing the `Exception.Message` is NOT the correct way to detect specific errors - in your example, try using `on E: EIdResolveError do` instead), you are accessing the UI labels without synchronizing with the UI thread. I suggest removing the `try..except` from `HttpGet()` altogether. Let each thread catch errors and sync with its corresponding label as needed. Then you can remove the `tip` parameter from `HttpGet()`. – Remy Lebeau Jun 21 '18 at 19:06
  • @DavidHeffernan fine, we can agree that a `try..finally` around `FreeOnTerminate` is wrong. – Remy Lebeau Jun 21 '18 at 19:14
  • @RemyLebeau ok, i edited and added to post with TThread.CreateAnonymousThread() its working,but it still eating memory about 1MB / 1 minute with timer interval 5 seconds, if i stop timer eating stop and value stay until i strart timer again. Any idea? Httpget i will fix later or you think problem can be there? – BOB Jun 21 '18 at 21:32
  • @BOB you MUST sync ALL access to the UI, or bad things happen. If an exception is raised, you MUST sync updates to your labels. If that doesn't fix the issue, you will just have to debug your code and track down where the leaks are actually occurring. Consider commenting out `http.Get()` and see what happens. If the leaks disappear then step into `Get()`'s source code with the debugger. However, the numbers you mention suggest that you are leaking ~28.4K per thread, but the code shown should not be using that much memory when downloading a measly 150 bytes, even when it gets decoded to Unicode – Remy Lebeau Jun 21 '18 at 21:56
  • What happens when you enable `ReportMemoryLeaksOnShutdown`? Does it report anything? What does it report? – Jerry Dodge Jun 21 '18 at 22:25
  • @JerryDodge my app is for android, and this working just for win or not? – BOB Jun 22 '18 at 04:21
  • @RemyLebeau now i did test: resault := same data (string) like are in url file, rest is working same, and it have 67MB and keep same value, and one more think, i have same function on win app but there is not problem with eating memory, that mean proglem is this: Result:=http.Get(url); – BOB Jun 22 '18 at 04:30
  • @RemyLebeau now i did next test, i downloaded file that i using for htpget, uploaded at another http server, and used new link for httpget and app size in ram weve constant, so its not about data but it does server where is file located, file i need is "https" but mine testing were on "http", EIdResolveError i found nothing on google about it, and if i delete try ... except from httpget i dont know how else i can get errors from httpget function – BOB Jun 22 '18 at 20:28
  • @RemyLebeau WOW you were working or still working at Embacadero? I see your name in IdGlobal.pas and about Indy very offten i found this: can be this mine problem? [link](https://stackoverflow.com/questions/5260680/why-are-memory-leaks-reported-for-indy-10) but it why you asked on delphi version so answer is no? – BOB Jun 22 '18 at 21:34
  • @BOB I am the primarily developer of Indy, yes. But I do not, and have never, worked for Embarcadero. And no, [those leaks](https://stackoverflow.com/questions/5260680/) have nothing to do with your issue. They are one-time leaks at program shutdown. – Remy Lebeau Jun 22 '18 at 21:41

0 Answers0