I've been trying to track down a memory leak in Jedi VCL's JvHidControllerClass.pas
, which i came across this change in the source history:
Older revision:
constructor TJvHidDeviceReadThread.CtlCreate(const Dev: TJvHidDevice);
begin
inherited Create(True);
Device := Dev;
NumBytesRead := 0;
SetLength(Report, Dev.Caps.InputReportByteLength);
end;
Current revision:
constructor TJvHidDeviceReadThread.CtlCreate(const Dev: TJvHidDevice);
begin
inherited Create(False);
Device := Dev;
NumBytesRead := 0;
SetLength(Report, Dev.Caps.InputReportByteLength);
end;
From experience, i've discovered that if you create a thread not suspended:
inherited Create(False);
then the thread immediately starts running. In this case it will attempt to access an object that has not been initialized yet:
procedure TJvHidDeviceReadThread.Execute;
begin
while not Terminated do
begin
FillChar(Report[0], Device.Caps.InputReportByteLength, #0);
if Device.ReadFileEx(Report[0], Device.Caps.InputReportByteLength, @DummyReadCompletion) then
right away trying to fill Report
, and access the object Device
. Problem is that they haven't been initialized yet; those are the next lines after the thread has started:
Device := Dev;
NumBytesRead := 0;
SetLength(Report, Dev.Caps.InputReportByteLength);
i realize this is a race condition; and the chances of a user experiencing a crash in production are pretty low, so it's probably harmless to leave the race-crash.
But am i way off? Am i missing something? Does calling:
BeginThread(nil, 0, @ThreadProc, Pointer(Self), Flags, FThreadID);
not start a thread off and running right away? Is this really a race condition regression that was (intentionally) added to JVCL? Is there some secret about
CreateSuspended(False);
that makes it the correct code over:
CreateSuspended(True);
...
FDataThread.Resume;
?
After having been burned by mistakenly calling
TMyThread.Create(False)
i've filed it in my brain as never correct. Is there any valid use for letting a thread start right away (when you have to initialize values)?