Page 1 of 1

Possible memory leak in TZCodePageConversionStream.Create

Posted: 23.01.2021, 18:18
by aehimself
Today it seems I managed to call .AbortOperation in a lucky spot, as I got a memory leak caused by GetMem:
leak.PNG
I think they should be encapsulated by this:

Code: Select all

  if (SourceCodePage = DestCodePage) then begin
    ReadStreamToMem(Src, L, Owner);
    Try
      SetPointer(Src, L);
      FInConstructionState := True;
    FInally
      Capacity := L; //realloc mem
      FInConstructionState := False;
    End;
  end else begin
But I'm not good with pointers and don't really know what is going on here... but it can be a starting point :)

Re: Possible memory leak in TZCodePageConversionStream.Create

Posted: 24.01.2021, 07:38
by EgonHugeist
aehimself

looking to the stacktrace.. I think there is a general design problem in what you're doing i guess.

Why is there an FetchAll called while you wanna kill the fetches dynamic?
Shouldn't you add an event to the dataset and test if it was signaled? I.e.

Code: Select all

While not Aborted do DataSet.Next
?

IIRC did the AbortOperation kill a long query while waiting for server responce. You StackTrace shows me the Server is ready and you're already consuming the inputs. That way is always dangerous and of course it leads to leaking memory. It's a random game where it happens.. Yet it are the lob's later it may be the accessor objects, or the fields or ... etc.

Re: Possible memory leak in TZCodePageConversionStream.Create

Posted: 24.01.2021, 09:09
by aehimself
EgonHugeist wrote: 24.01.2021, 07:38Why is there an FetchAll called while you wanna kill the fetches dynamic?
This worker thread collects a large amount of CLOB fields and does some calculations on their contents one-by-one. If I call .FetchAll before the actual data processing 2 things will happen:
- The application will report nothing but "Opening query" for ~20-30 minutes instead of almost immediately showing that it already processed some
- Memory consumption will be in the 4 GB-ish range instead of 150 MB
EgonHugeist wrote: 24.01.2021, 07:38looking to the stacktrace.. I think there is a general design problem in what you're doing i guess.
[...]
Shouldn't you add an event to the dataset and test if it was signaled? I.e.

Code: Select all

While not Aborted do DataSet.Next
I do have a cycle like this... in this case the question is valid... why am I calling .AbortOperation at the first place...? You got me thinking, I'll attempt to get around it.

The reason I reported is because in the constructor there is an if statement. One side has a Try..Finally block, the other does not and that looked suspicious.
Just take a look when you have some free time; I'll attempt to eliminate the .AbortOperation in the mean time.