TThread.Synchronize - undefined behaviour / crash
Original Reporter info from Mantis: Martin @martin_frb
-
Reporter name: Martin Friebe
Original Reporter info from Mantis: Martin @martin_frb
- Reporter name: Martin Friebe
Description:
From a review of the source, there may be issue:
I found and tested the issue with 3.0.4 on win32. But a review of 3.2 branch shows that the problem is still there.
TThread.Synchronize
will call
ThreadQueueAppend(AThread.FSynchronizeEntry);
AThread.FSynchronizeEntry is the re-usable entry
(which is fine, only one synchronize call can be active in any given thread. (Unless you are in a synchronize event, and call MyWaitingThread.synchronize from inside the event....))
ThreadQueueAppend puts the FSynchronizeEntry at the end of the queue.
It does NOT clear FSynchronizeEntry^.Next, it expects it to be nil already. => TThread.Synchronize will reset Next to nil after ThreadQueueAppend returns
ThreadQueueAppend waits for the synchronize call to finish.
But, if the sync event threw an exception, this exception is returned to the thread.
ThreadQueueAppend checks if an exception was returned and raises it.
In case of such an exception TThread.Synchronize will never get to execute
AThread.FSynchronizeEntry^.Next := Nil;
The next time that thread calls TThread.Synchronize, the Next pointer is dirty.
--------------------
The code in "steps to reproduce" demonstrates this (cheaply using "sleep" to ensure threads are executed in desired order....)
MySync2 Is called only once, yet the output is
x
x2
x
x2
If Thread2 had been free'd, and the memory for its FSynchronizeEntry had been reused with other data, then this would likely cause a crash.
Steps to reproduce:
program Project1;
{$mode objfpc}{$H+}
uses
{$IFDEF UNIX}{$IFDEF UseCThreads} cthreads, {$ENDIF}{$ENDIF}
Classes, sysutils;
type
MT1= class(TThread)
procedure Execute; override;
private
procedure MySync;
end;
{ MT2 }
MT2= class(TThread)
procedure Execute; override;
private
procedure MySync2;
end;
var
T1: MT1;
T2: MT2;
{ MT2 }
procedure MT2.Execute;
begin
try
Synchronize(@MySync2);
except end;
while true do sleep(100*1000);
end;
procedure MT2.MySync2;
begin
writeln('x2 ');
raise Exception.Create('Foo'); // prevent event^.Method from being set to nil
end;
procedure MT1.Execute;
begin
try
Synchronize(@MySync);
except end;
Sleep(10*1000);
try
Synchronize(@MySync);
except end;
while true do sleep(100*1000);
end;
procedure MT1.MySync;
begin
writeln('x');
raise Exception.Create('Foo'); // prevent event^.Next from being set to nil
end;
begin
T1 := MT1.Create(False);
Sleep(2*1000);
T2 := MT2.Create(False);
Sleep(2*1000);
CheckSynchronize(30*1000);
CheckSynchronize(30*1000);
CheckSynchronize(30*1000);
CheckSynchronize(30*1000);
readln;
end.
Mantis conversion info:
- Mantis ID: 35027
- OS: win 10
- OS Build: 10
- Platform: 64bit Intel
- Version: 3.2.0
- Fixed in version: 3.3.1
- Fixed in revision: 41281 (#b810d8f3)
- Monitored by: » crossbuilder (Burkhard Carstens), » AntonK (Anton Kavalenka)