OtlParallel Memory Leak

[Celebr0 2]

Hi, for some reason, I'm experiencing massive memory leaks when reading a file and parallelizing by lines

 

procedure ReadF(const fname: string);
const
BUFSIZE = 1024 * 18;
var
Line:string;
List: TStringList;
Reader: TStreamReader;
Buf: array [0..BUFSIZE] of char;
begin
List := TStringList.Create;
Reader := TStreamReader.Create(fname, TEncoding.Default, True, 4096);
while not Reader.EndOfStream do
begin
Reader.ReadBlock(@Buf, 0, BUFSIZE);
List.Text := Buf;
parallel.&For(0, List.Count - 1).Execute(
procedure(value: integer)
begin
Line:=List[value];
end);
end;
List.Free;
Reader.Close;
FreeAndNil(Reader);
end;

 

[Vandrovnik 217]

Assigning to one string variable from multiple threads at same time is not a good idea...

[Celebr0 2]

21 minutes ago, Vandrovnik said:

Assigning to one string variable from multiple threads at same time is not a good idea...

Who told you such nonsense? The variable will just be overwritten!

 

[Dalija Prasnikar 1478]

40 minutes ago, Celebr0 said:

Who told you such nonsense? The variable will just be overwritten!

Actually it is not nonsense at all. This is the source of your memory leaks. String assignments are not thread-safe.

[Celebr0 2]

16 minutes ago, Dalija Prasnikar said:

Actually it is not nonsense at all. This is the source of your memory leaks. String assignments are not thread-safe.

Working with memory, since it is fast, is always absolutely thread-safe. However, working with form1 for example Memo1, ListBox, and so on is not thread-safe. If you are implying that I should wrap Line := List[value]; in a critical section, it makes absolutely no difference—memory leaks still remain.

 

Edited Tuesday at 07:38 PM by Celebr0

[Dalija Prasnikar 1478]

50 minutes ago, Celebr0 said:

Working with memory, since it is fast, is always absolutely thread-safe

Thread safety has nothing to to with speed. Strings are reference-counted and string assignment is not atomic operation. Assigning string variable from multiple threads can mess up the reference count and cause memory leaks and crashes. 

 

50 minutes ago, Celebr0 said:

If you are implying that I should wrap Line := List[value]; in a critical section, it makes absolutely no difference—memory leaks still remain.

Yes. Access to the string needs to be protected (that includes other code that accesses that string even if it is just being read from). But, if the leaks are still there, then there are most likely some problems in other code you haven't posted.

 

Which Delphi version are you using?

[Celebr0 2]

1 hour ago, Dalija Prasnikar said:

Thread safety has nothing to to with speed. Strings are reference-counted and string assignment is not atomic operation. Assigning string variable from multiple threads can mess up the reference count and cause memory leaks and crashes. 

 

Yes. Access to the string needs to be protected (that includes other code that accesses that string even if it is just being read from). But, if the leaks are still there, then there are most likely some problems in other code you haven't posted.

 

Which Delphi version are you using?

Delphi XE2 

I wrote my own module for parallelizing threads using the same code as here, and there are no memory leaks at all!

 

[Tommi Prami 140]

12 hours ago, Celebr0 said:

Delphi XE2 

I wrote my own module for parallelizing threads using the same code as here, and there are no memory leaks at all!

If you write and/or access same string from multiple threads, is not thread safe. 

If you did not get errors or memory leaks is pure luck, or you did not check memory leaks with tool, that actually will report you the actual horror of updating strigg from multiple thread without proper protection (not a condom)

-Tee-

[Roger Cigol 128]

1 hour ago, Tommi Prami said:

If you did not get errors or memory leaks is pure luck

Yes: one of the worst things about thread safe / non-safe issues is that they do not always show themselves. A ThreadSafety issue can remain unnoticed during all your development and then you send it off to your customer and it falls over immediately. It has happened to me. Any thread work needs very careful design so that such bugs (which are the hardest to find) are avoided in the first place.

[Celebr0 2]

On 3/26/2025 at 4:15 PM, Roger Cigol said:

Yes: one of the worst things about thread safe / non-safe issues is that they do not always show themselves. A ThreadSafety issue can remain unnoticed during all your development and then you send it off to your customer and it falls over immediately. It has happened to me. Any thread work needs very careful design so that such bugs (which are the hardest to find) are avoided in the first place.

I think I figured it out when I was writing my own module for parallelizing the for loop, this is what I encountered:

 

1.

There will be memory leaks:

 

procedure ParallelFor(const AStart, AEnd: Integer; AProc: TProc<Integer>);
var
i, ThreadCount, RangeStart: Integer;
begin
ThreadCount:=GetCPUCount;
RangeStart:=AStart-1;
for i := 0 to ThreadCount - 1 do
WaitForSingleObject(TThread.CreateAnonymousThread(
procedure
begin
while RangeStart < AEnd do begin
AProc(InterlockedIncrement(RangeStart));
end;
end).Create(False).Handle, INFINITE);
end;

 

2. 

There will be no memory leaks here:

 

procedure ParallelFor(const AStart, AEnd: Integer; AProc: TProc<Integer>);
var
i, ThreadCount, RangeStart: Integer;
begin
ThreadCount:=GetCPUCount;
RangeStart:=AStart-1;
for i := 0 to ThreadCount - 1 do
TThread.CreateAnonymousThread(
procedure
begin
while RangeStart < AEnd do begin
AProc(InterlockedIncrement(RangeStart));
end;
end).Create(False);
end;

 

It looks like OtlParallel.For is written based on the same principle as in the first case !

 

[Celebr0 2]

I checked and solved this issue, and yes, that is indeed the case:

 

Memory Leak:

 

procedure ReadF(const fname: string);
const
BUFSIZE = 1024 * 18;
var
Line:string;
List: TStringList;
Reader: TStreamReader;
Buf: array [0..BUFSIZE] of char;
begin
List := TStringList.Create;
Reader := TStreamReader.Create(fname, TEncoding.Default, True, 4096);
while not Reader.EndOfStream do
begin
Reader.ReadBlock(@Buf, 0, BUFSIZE);
List.Text := Buf;
parallel.ForEach(0, List.Count - 1).Execute(
procedure(value: integer)
begin
Line:=List[value];
end);
end;
List.Free;
Reader.Close;
FreeAndNil(Reader);
end;

 

No memory Leak:

 

procedure ReadF(const fname: string);
const
BUFSIZE = 1024 * 18;
var
Line:string;
List: TStringList;
Reader: TStreamReader;
Buf: array [0..BUFSIZE] of char;
begin
List := TStringList.Create;
Reader := TStreamReader.Create(fname, TEncoding.Default, True, 4096);
while not Reader.EndOfStream do
begin
Reader.ReadBlock(@Buf, 0, BUFSIZE);
List.Text := Buf;
parallel.ForEach(0, List.Count - 1).NoWait.Execute(
procedure(value: integer)
begin
Line:=List[value];
end);
end;
List.Free;
Reader.Close;
FreeAndNil(Reader);
end;

[Tommi Prami 140]

12 hours ago, Celebr0 said:

I checked and solved this issue, and yes, that is indeed the case:

...

 

No memory Leak:

....

parallel.ForEach(0, List.Count - 1).NoWait.Execute(
...

I bet you did not fix the actual problem here.

-Tee-

[Celebr0 2]

14 hours ago, Tommi Prami said:

I bet you did not fix the actual problem here.

-Tee-

How much are you betting ?