Recent

Author Topic: Main thread is being blocked by another thread and WaitForSingleObject  (Read 8387 times)

stoffman

  • Jr. Member
  • **
  • Posts: 67
Hey,

I've created a thread which creates a process and waits for this process to end. I expected that it will not block the main thread and the UI will remain responsive. But that is not the case...
Any ideas ?



Code: Pascal  [Select][+][-]
  1. TExternalProcess = class(TThread)
  2.  
  3. ...
  4. procedure TExternalProcess.Execute;
  5. begin
  6.   if CreateProcess(nil, PChar(fExe), @Security, @Security, true, NORMAL_PRIORITY_CLASS or $08000000,
  7.              nil, nil, start2, ProcessInfo)
  8.       then begin
  9.           WaitForSingleObject (ProcessInfo.hProcess,Infinite);
  10.           ....
  11.           ....
  12.       end;
  13. end;
  14.  
  15. procedure TForm1.Button1Click(Sender: TObject);
  16. var
  17.   APRocess : TExternalProcess;
  18. begin
  19.   AProcess := TExternalProcess.Create(true);
  20.   AProcess.Execute;
  21. end;
  22.  



Thanks,
Yoni

HeavyUser

  • Sr. Member
  • ****
  • Posts: 397
Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. var
  3.   APRocess : TExternalProcess;
  4. begin
  5.   AProcess := TExternalProcess.Create(true);
  6.   AProcess.Execute;
  7. end;
  8.  
Yeah you do not call the execute method directly you simply resume the thread eg
Code: Pascal  [Select][+][-]
  1. procedure TForm1.Button1Click(Sender: TObject);
  2. var
  3.   APRocess : TExternalProcess;
  4. begin
  5.   AProcess := TExternalProcess.Create(true);
  6.   AProcess.Resume;
  7. end;
  8.  
That should solve the problem of pausing the main thread.

Thaddy

  • Hero Member
  • *****
  • Posts: 14197
  • Probably until I exterminate Putin.
Indeed. AProcess.Execute;? That is supposed to work only IN the thread. You called the execute method of the thread in the context of the main process.
Just set CreateSuspended to false instead of true in the constructor, or use in place of Execute, Resume (Note that is actually deprecated)
Specialize a type, not a var.

Thaddy

  • Hero Member
  • *****
  • Posts: 14197
  • Probably until I exterminate Putin.
As a sidenote, I found a brilliant example that demonstrates why you should not use suspend resume.
It is from Barry Kelly (formerly Delphi's senior compiler engineer), I adapted it and made it cross-platform.
The problem is universal, not Delphi related, not platform related, not CPU related and simply a design flaw.

Run the program under Windows or Linux (I checked on arm-linux too...) It will (fail, but not crash) end up with deadlocks....
Just give it 30 secs or so to "recover" from the deadlocks.

Maybe I should add it to the wiki?
Code: Pascal  [Select][+][-]
  1. Program BadNews;
  2. //This program is adapted from Barry Kelly's example
  3. //from http://codeverge.com/embarcadero.delphi.win32/threads/1039644
  4. // I made it cross platform
  5. // It is a demonstration of why you should not use
  6. // TThread.Suspend/TThread.Resume, unless you are writing a debugger.
  7. //
  8. // Op.Cit Barry Kelly:
  9. // --- Folks, just don't use TThread.Suspend. If you still think you need it,
  10. // you had better be implementing a debugger, GC,
  11. // or something equivalently low-level.
  12. // -- Barry
  13. //
  14. // Note the problem is universal and not limited to Delphi, as you will
  15. // find out when you compile and run the program a few times ;)
  16. // Thaddy
  17. {$mode delphi}
  18. {$ifdef mswindows}{$APPTYPE CONSOLE}{$endif}
  19.  
  20. Uses
  21. {$ifdef unix}cthreads,{$else}
  22.   {$ifdef msWindows}
  23.    windows,
  24.    {$endif}
  25. {$endif}
  26.   SysUtils, Classes, SyncObjs;
  27.  
  28. Type TMyThread = Class(TThread)
  29.   Private FCritSec: TCriticalSection;
  30.     FList: TList;
  31.   Protected Procedure Execute;
  32.     override;
  33.   Public constructor Create;
  34.     destructor Destroy;
  35.     override;
  36.     Procedure DoWork(Var x: Integer);
  37. End;
  38.  
  39. Var IOCritSec: TCriticalSection;
  40.   // Thread-safe console output
  41. Procedure TWrite(Const s: String);
  42. Begin
  43.   IOCritSec.Acquire;
  44.   Try
  45.     Writeln(Output, s);
  46.     Flush(Output);
  47.   Finally
  48.     IOCritSec.Release;
  49. End;
  50. End;
  51.  { TMyThread }
  52. constructor TMyThread.Create;
  53. Begin
  54.   inherited Create(False);
  55.   FCritSec := TCriticalSection.Create;
  56.   FList := TList.Create;
  57. End;
  58. destructor TMyThread.Destroy;
  59. Begin
  60.   FCritSec.Free;
  61.   FList.Free;
  62.   inherited;
  63. End;
  64.  
  65. Procedure TMyThread.DoWork(Var x: Integer);
  66. Begin
  67.   FCritSec.Acquire;
  68.   Try
  69.     FList.Insert(0, @x)
  70.   Finally
  71.     FCritSec.Release;
  72. End;
  73. If Suspended Then Resume;
  74. End;
  75.  
  76. Procedure TMyThread.Execute;
  77.  
  78. Var work: PInteger;
  79. Begin
  80.   While Not Terminated Do
  81.     Begin
  82.       TWrite('W');
  83.       // heartbeat indicating OK
  84.       FCritSec.Acquire;
  85.       Try
  86.         If FList.Count > 0 Then
  87.           Begin
  88.             work :=
  89.                     FList.Last;
  90.             FList.Delete(FList.Count - 1);
  91.           End
  92.         Else
  93.           work := Nil;
  94.       Finally
  95.         FCritSec.Release;
  96.     End;
  97.   If work = Nil Then
  98.     Begin
  99.       TWrite('-sleep-');
  100.       Suspend;
  101.     End
  102.   Else
  103.     Begin
  104.       Inc(work^);
  105.     End;
  106. End;
  107. End;
  108.  
  109. Procedure Produce;
  110.  
  111. Var worker: TMyThread;
  112. Procedure SpinUntil(Var ref: Integer; val: Integer);
  113.  
  114. Var maxSpin: Integer;
  115. Begin
  116.   maxSpin := High(Integer);
  117.   While ref <> val Do
  118.     Begin
  119.       Dec(maxSpin);
  120.       If maxSpin = 0 Then
  121.         Begin
  122.           If worker.Suspended Then
  123.             Begin
  124.               TWrite('Oh dear: we deadlocked.');
  125.               TWrite(IntToStr(worker.FList.Count) + ' missed work items.');
  126.               Halt;
  127.             End
  128.           Else
  129.             TWrite('Unexpected spin loop failure');
  130.           Exit;
  131.         End;
  132.     End;
  133. End;
  134.  
  135. Var a, b, c: Integer;
  136. Begin
  137.   worker := TMyThread.Create;
  138.   Try
  139.     While True Do
  140.       Begin
  141.         a := 10;
  142.         b := 20;
  143.         c := 30;
  144.         worker.DoWork(a);
  145.         worker.DoWork(b);
  146.         worker.DoWork(c);
  147.         SpinUntil(c, 31);
  148.         SpinUntil(a, 11);
  149.         SpinUntil(b, 21);
  150.         TWrite('P');
  151.         // heartbeat indicating OK
  152.       End;
  153.   Finally
  154.     worker.Free;
  155. End;
  156. End;
  157.  
  158. Begin
  159.   IOCritSec := TCriticalSection.Create;
  160.   Try
  161.     Try
  162.       Produce;
  163.     Except
  164.       on E: Exception Do Writeln(E.Classname, ': ', E.Message);
  165. End;
  166. Finally
  167.   IOCritSec.Free;
  168. End;
  169. End.
« Last Edit: June 25, 2017, 09:45:14 pm by Thaddy »
Specialize a type, not a var.

stoffman

  • Jr. Member
  • **
  • Posts: 67
Oh my! I didn't notice the execute...

Thank you all for the help!

@Thaddy Resume is deprecated for that reason. Start is the new Resume :-) 

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11382
  • FPC developer.
Yes. "start" is just an alias for resume without deprecated warning. Delphi did this because resume to undo .suspend, and resume to start a thread that was created suspended are two different things. The first (suspend/resume pairs) are deprecated, the second (starting a thread that was created suspended) not.

So Thaddy is talking about a different thing. Using resume to start a thread created as suspended will give a harmless warning, but is not fundamentally wrong.

Thaddy

  • Hero Member
  • *****
  • Posts: 14197
  • Probably until I exterminate Putin.
No, I am talking about the same thing: Although resume is not the culprit and basically aliased to the "new" start, Suspend definitely is..
Since these two are semantically (natural language wise) too closely related the Delphi guys wisely chose to leave that pair alone.
If you come from a native English speaking background this is much easier to understand.

The problems all stem from using suspend. That is clearly demonstrated by the code from Barry Kelly, which in itself contains not a single error.
Just don't use suspend/resume. Ok, if you must, use start,but don't use suspend. Ever. Use a TEvent object or a semaphore.
(Unless you are writing debuggers that is).
« Last Edit: June 27, 2017, 03:14:05 pm by Thaddy »
Specialize a type, not a var.

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11382
  • FPC developer.
No, I am talking about the same thing: Although resume is not the culprit and basically aliased to the "new" start, Suspend definitely is..

And nobody talked about suspend but you. 
 

Thaddy

  • Hero Member
  • *****
  • Posts: 14197
  • Probably until I exterminate Putin.
Marco,
I merely wrote "sidenote" which is fully reasonable within the context. There is little explanation available WHY the pair suspend/resume are deprecated.
I just added code  that proves why not. And that's within context.
Specialize a type, not a var.

 

TinyPortal © 2005-2018