With what delphi Code should I replace my calls to deprecated TThread method Suspend?
Asked Answered
D

4

16

It has been asked before, but without a full answer. This is to do with the so called famous "‘Fatal threading model!’".

I need to replace this call to TThread.Suspend with something safe, that returns when terminated or resumed:

procedure TMyThread.Execute;
begin
  while (not Terminated) do begin
     if PendingOffline then begin
          PendingOffline := false;   // flag off.
          ReleaseResources;
          Self.Suspend; // suspend thread. { evil! ask Barry Kelly why.}
          // -- somewhere else, after a long time, a user clicks
          // a resume button, and the thread resumes: --
          if Terminated then
              exit; // leave TThread.Execute.
          // Not terminated, so we continue..
          GrabResources;
     end;
    end;
end;

The original answer vaguely suggests "TMutex, TEvent and critical sections".

I guess I'm looking for a TThreadThatDoesntSuck.

Here's the sample TThread derivative with a Win32Event, for comments:

unit SignalThreadUnit;

interface

uses
  Classes,SysUtils,Windows;

type

TSignalThread = class(TThread)
  protected
    FEventHandle:THandle;
    FWaitTime :Cardinal; {how long to wait for signal}
    //FCritSec:TCriticalSection; { critical section to prevent race condition at time of change of Signal states.}
    FOnWork:TNotifyEvent;

    FWorkCounter:Cardinal; { how many times have we been signalled }

    procedure Execute; override; { final; }

    //constructor Create(CreateSuspended: Boolean); { hide parent }
  public
    constructor Create;
    destructor Destroy; override;

    function WaitForSignal:Boolean; { returns TRUE if signal received, false if not received }

    function Active:Boolean; { is there work going on? }

    property WorkCounter:Cardinal read FWorkCounter; { how many times have we been signalled }

    procedure Sync(AMethod: TThreadMethod);

    procedure Start; { replaces method from TThread }
    procedure Stop; { provides an alternative to deprecated Suspend method }

    property Terminated; {make visible}

  published
      property WaitTime :Cardinal read FWaitTime write FWaitTime; {how long to wait for signal}

      property OnWork:TNotifyEvent read FOnWork write FOnWork;

end;

implementation

{ TSignalThread }

constructor TSignalThread.Create;
begin
  inherited Create({CreateSuspended}true);
 // must create event handle first!
  FEventHandle := CreateEvent(
          {security}      nil,
          {bManualReset}  true,
          {bInitialState} false,
          {name}          nil);

  FWaitTime := 10;
end;

destructor TSignalThread.Destroy;
begin
 if Self.Suspended or Self.Terminated then
    CloseHandle(FEventHandle);
  inherited;
end;



procedure TSignalThread.Execute;
begin
//  inherited; { not applicable here}
  while not Terminated do begin
      if WaitForSignal then begin
          Inc(FWorkCounter);
          if Assigned(FOnWork) then begin
              FOnWork(Self);
          end;
      end;
  end;
  OutputDebugString('TSignalThread shutting down');

end;

{ Active will return true when it is easily (instantly) apparent that
  we are not paused.  If we are not active, it is possible we are paused,
  or it is possible we are in some in-between state. }
function TSignalThread.Active: Boolean;
begin
 result := WaitForSingleObject(FEventHandle,0)= WAIT_OBJECT_0;
end;

procedure TSignalThread.Start;
begin
  SetEvent(FEventHandle); { when we are in a signalled state, we can do work}
  if Self.Suspended then
      inherited Start;

end;

procedure TSignalThread.Stop;
begin
    ResetEvent(FEventHandle);
end;

procedure TSignalThread.Sync(AMethod: TThreadMethod);
begin
 Synchronize(AMethod);
end;

function TSignalThread.WaitForSignal: Boolean;
var
 ret:Cardinal;
begin
  result := false;
  ret := WaitForSingleObject(FEventHandle,FWaitTime);
  if (ret=WAIT_OBJECT_0) then
      result := not Self.Terminated;
end;

end.
Dierolf answered 19/1, 2010 at 21:40 Comment(4)
For a TThreadThatDoesntSuck you should have a look at the OmniThreadLibrary, otl.17slon.comWatanabe
What happens when the code that does "the thread resumes" (thread.Resume) before "Self.Suspend" occurs? What you actually want is a manual reset event which is set to unsignalled, wait for it where you would Suspend, and set the event where you would Resume. That way, you won't miss a Resume by Suspending too late.Sorgo
I think Barry's comment is pretty close to being my answer. I edited the question to show a sample implementation of a 'suspend-less' way of pausing (blocking) a thread until I want it to use up some CPU time again. Basically I want a "start" and "stop" abstraction. The thread should never terminate until the application is ending, and we are cleaning up.Dierolf
Interesting related attempt to create a "suspendable thread" in a dot-net environment, reveals that Microsoft giveth and taketh away the Thread.Suspend method also: msmvps.com/blogs/peterritchie/archive/2006/10/13/…2700_Thread.Suspend-has-been-deprecated_2E002E002E00.aspxDierolf
H
9

EDIT: Latest version can be found on GitHub: https://github.com/darianmiller/d5xlib

I've come up with this solution as a basis for TThread enhancement with a working Start/Stop mechanism that doesn't rely on Suspend/Resume. I like to have a thread manager that monitors activity and this provides some of the plumbing for that.

unit soThread;

interface

uses
  Classes,
  SysUtils,
  SyncObjs,
  soProcessLock;


type

  TsoThread = class;
  TsoNotifyThreadEvent = procedure(const pThread:TsoThread) of object;
  TsoExceptionEvent = procedure(pSender:TObject; pException:Exception) of object;


  TsoThreadState = (tsActive,
                    tsSuspended_NotYetStarted,
                    tsSuspended_ManuallyStopped,
                    tsSuspended_RunOnceCompleted,
                    tsTerminationPending_DestroyInProgress,
                    tsSuspendPending_StopRequestReceived,
                    tsSuspendPending_RunOnceComplete,
                    tsTerminated);

  TsoStartOptions = (soRepeatRun,
                     soRunThenSuspend,
                     soRunThenFree);



  TsoThread = class(TThread)
  private
    fThreadState:TsoThreadState;
    fOnException:TsoExceptionEvent;
    fOnRunCompletion:TsoNotifyThreadEvent;
    fStateChangeLock:TsoProcessResourceLock;
    fAbortableSleepEvent:TEvent;
    fResumeSignal:TEvent;
    fTerminateSignal:TEvent;
    fExecDoneSignal:TEvent;
    fStartOption:TsoStartOptions;
    fProgressTextToReport:String;
    fRequireCoinitialize:Boolean;
    function GetThreadState():TsoThreadState;
    procedure SuspendThread(const pReason:TsoThreadState);
    procedure Sync_CallOnRunCompletion();
    procedure DoOnRunCompletion();
    property ThreadState:TsoThreadState read GetThreadState;
    procedure CallSynchronize(Method: TThreadMethod);
  protected
    procedure Execute(); override;

    procedure BeforeRun(); virtual;      // Override as needed
    procedure Run(); virtual; ABSTRACT;  // Must override
    procedure AfterRun(); virtual;       // Override as needed

    procedure Suspending(); virtual;
    procedure Resumed(); virtual;
    function ExternalRequestToStop():Boolean; virtual;
    function ShouldTerminate():Boolean;

    procedure Sleep(const pSleepTimeMS:Integer);  

    property StartOption:TsoStartOptions read fStartOption write fStartOption;
    property RequireCoinitialize:Boolean read fRequireCoinitialize write fRequireCoinitialize;
  public
    constructor Create(); virtual;
    destructor Destroy(); override;

    function Start(const pStartOption:TsoStartOptions=soRepeatRun):Boolean;
    procedure Stop();  //not intended for use if StartOption is soRunThenFree

    function CanBeStarted():Boolean;
    function IsActive():Boolean;

    property OnException:TsoExceptionEvent read fOnException write fOnException;
    property OnRunCompletion:TsoNotifyThreadEvent read fOnRunCompletion write fOnRunCompletion;
  end;


implementation

uses
  ActiveX,
  Windows;


constructor TsoThread.Create();
begin
  inherited Create(True); //We always create suspended, user must call .Start()
  fThreadState := tsSuspended_NotYetStarted;
  fStateChangeLock := TsoProcessResourceLock.Create();
  fAbortableSleepEvent := TEvent.Create(nil, True, False, '');
  fResumeSignal := TEvent.Create(nil, True, False, '');
  fTerminateSignal := TEvent.Create(nil, True, False, '');
  fExecDoneSignal := TEvent.Create(nil, True, False, '');
end;


destructor TsoThread.Destroy();
begin
  if ThreadState <> tsSuspended_NotYetStarted then
  begin
    fTerminateSignal.SetEvent();
    SuspendThread(tsTerminationPending_DestroyInProgress);
    fExecDoneSignal.WaitFor(INFINITE); //we need to wait until we are done before inherited gets called and locks up as FFinished is not yet set
  end;
  inherited;
  fAbortableSleepEvent.Free();
  fStateChangeLock.Free();
  fResumeSignal.Free();
  fTerminateSignal.Free();
  fExecDoneSignal.Free();
end;


procedure TsoThread.Execute();

            procedure WaitForResume();
            var
              vWaitForEventHandles:array[0..1] of THandle;
              vWaitForResponse:DWORD;
            begin
              vWaitForEventHandles[0] := fResumeSignal.Handle;
              vWaitForEventHandles[1] := fTerminateSignal.Handle;
              vWaitForResponse := WaitForMultipleObjects(2, @vWaitForEventHandles[0], False, INFINITE);
              case vWaitForResponse of
              WAIT_OBJECT_0 + 1: Terminate;
              WAIT_FAILED: RaiseLastOSError;
              //else resume
              end;
            end;
var
  vCoInitCalled:Boolean;
begin
  try
    try
      while not ShouldTerminate() do
      begin
        if not IsActive() then
        begin
          if ShouldTerminate() then Break;
          Suspending;
          WaitForResume();   //suspend()

          //Note: Only two reasons to wake up a suspended thread:
          //1: We are going to terminate it  2: we want it to restart doing work
          if ShouldTerminate() then Break;
          Resumed();
        end;

        if fRequireCoinitialize then
        begin
          CoInitialize(nil);
          vCoInitCalled := True;
        end;
        BeforeRun();
        try
          while IsActive() do
          begin
            Run(); //descendant's code
            DoOnRunCompletion();

            case fStartOption of
            soRepeatRun:
              begin
                //loop
              end;
            soRunThenSuspend:
              begin
                SuspendThread(tsSuspendPending_RunOnceComplete);
                Break;
              end;
            soRunThenFree:
              begin
                FreeOnTerminate := True;
                Terminate();
                Break;
              end;
            else
              begin
                raise Exception.Create('Invalid StartOption detected in Execute()');
              end;
            end;
          end;
        finally
          AfterRun();
          if vCoInitCalled then
          begin
            CoUnInitialize();
          end;
        end;
      end; //while not ShouldTerminate()
    except
      on E:Exception do
      begin
        if Assigned(OnException) then
        begin
          OnException(self, E);
        end;
        Terminate();
      end;
    end;
  finally
    //since we have Resumed() this thread, we will wait until this event is
    //triggered before free'ing.
    fExecDoneSignal.SetEvent();
  end;
end;


procedure TsoThread.Suspending();
begin
  fStateChangeLock.Lock();
  try
    if fThreadState = tsSuspendPending_StopRequestReceived then
    begin
      fThreadState := tsSuspended_ManuallyStopped;
    end
    else if fThreadState = tsSuspendPending_RunOnceComplete then
    begin
      fThreadState := tsSuspended_RunOnceCompleted;
    end;
  finally
    fStateChangeLock.Unlock();
  end;
end;


procedure TsoThread.Resumed();
begin
  fAbortableSleepEvent.ResetEvent();
  fResumeSignal.ResetEvent();
end;


function TsoThread.ExternalRequestToStop:Boolean;
begin
  //Intended to be overriden - for descendant's use as needed
  Result := False;
end;


procedure TsoThread.BeforeRun();
begin
  //Intended to be overriden - for descendant's use as needed
end;


procedure TsoThread.AfterRun();
begin
  //Intended to be overriden - for descendant's use as needed
end;


function TsoThread.Start(const pStartOption:TsoStartOptions=soRepeatRun):Boolean;
var
  vNeedToWakeFromSuspendedCreationState:Boolean;
begin
  vNeedToWakeFromSuspendedCreationState := False;

  fStateChangeLock.Lock();
  try
    StartOption := pStartOption;

    Result := CanBeStarted();
    if Result then
    begin
      if (fThreadState = tsSuspended_NotYetStarted) then
      begin
        //Resumed() will normally be called in the Exec loop but since we
        //haven't started yet, we need to do it here the first time only.
        Resumed();
        vNeedToWakeFromSuspendedCreationState := True;
      end;

      fThreadState := tsActive;

      //Resume();
      if vNeedToWakeFromSuspendedCreationState then
      begin
        //We haven't started Exec loop at all yet
        //Since we start all threads in suspended state, we need one initial Resume()
        Resume();
      end
      else
      begin
        //we're waiting on Exec, wake up and continue processing
        fResumeSignal.SetEvent();
      end;
    end;
  finally
    fStateChangeLock.Unlock();
  end;
end;


procedure TsoThread.Stop();
begin
  SuspendThread(tsSuspendPending_StopRequestReceived);
end;


procedure TsoThread.SuspendThread(const pReason:TsoThreadState);
begin
  fStateChangeLock.Lock();
  try
    fThreadState := pReason; //will auto-suspend thread in Exec
    fAbortableSleepEvent.SetEvent();
  finally
    fStateChangeLock.Unlock();
  end;
end;


procedure TsoThread.Sync_CallOnRunCompletion();
begin
  if Assigned(fOnRunCompletion) then fOnRunCompletion(Self);
end;


procedure TsoThread.DoOnRunCompletion();
begin
  if Assigned(fOnRunCompletion) then CallSynchronize(Sync_CallOnRunCompletion);
end;


function TsoThread.GetThreadState():TsoThreadState;
begin
  fStateChangeLock.Lock();
  try
    if Terminated then
    begin
      fThreadState := tsTerminated;
    end
    else if ExternalRequestToStop() then
    begin
      fThreadState := tsSuspendPending_StopRequestReceived;
    end;
    Result := fThreadState;
  finally
    fStateChangeLock.Unlock();
  end;
end;


function TsoThread.CanBeStarted():Boolean;
begin
  Result := (ThreadState in [tsSuspended_NotYetStarted,
                             tsSuspended_ManuallyStopped,
                             tsSuspended_RunOnceCompleted]);
end;

function TsoThread.IsActive():Boolean;
begin
  Result := (ThreadState = tsActive);
end;


procedure TsoThread.Sleep(const pSleepTimeMS:Integer);
begin
  fAbortableSleepEvent.WaitFor(pSleepTimeMS);
end;


procedure TsoThread.CallSynchronize(Method: TThreadMethod);
begin
  if IsActive() then
  begin
    Synchronize(Method);
  end;
end;

Function TsoThread.ShouldTerminate():Boolean;
begin
  Result := Terminated or
            (ThreadState in [tsTerminationPending_DestroyInProgress, tsTerminated]);
end;

end.
Harmsworth answered 2/3, 2010 at 19:0 Comment(7)
TmyExampleLongTaskThread = class(TsoThread) protected procedure BeforeRun(); override; procedure Run; override; procedure AfterRun(); override; end; procedure TmyExampleLongTaskThread.BeforeRun(); begin MessageBeep(MB_ICONEXCLAMATION); end; procedure TmyExampleLongTaskThread.AfterRun(); begin MessageBeep(MB_ICONASTERISK); end; //repeatedly do some work that takes time.. procedure TmyExampleLongTaskThread.Run; begin self.Sleep(3000); end; .... Worker := TmyExampleLongTaskThread.Create(); fWorker.Start(soRepeatRun);Harmsworth
I'm marking this as an answer, more or less because I think that the thread class in the VCL should contain this mechanism already, and that marking one method deprecated and not giving a new mechanism to replace it completely, is cheezy. Some day, maybe they will fix TThread, so it's as "useful" as it used to be before they deprecated the broken bits.Dierolf
This code is working for me. I slimmed it down a bit to post a 'Tso' version as you wanted a complete class. The one available framework, OmniThreadLibrary, is a bit overkill for me. It's got to be understandable, debuggable, and usable and it's not readily dropped-into place. This particular class of mine is a nearly drop-in, working replacement for TThread and can be used with a usable Start/Stop mechanism. TThreadEX was the other name I was going to use to post it here.Harmsworth
I'm just starting a threading project and this looks like a great place for me to begin. Is soProcessLock around somewhere? That is needed to actually use the class you have posted above.Burrow
That's just a TRTLCriticalSection wrapper. Constructor calls InitializeCriticalSection, destructor calls DeleteCriticalSection. Lock calls EnterCriticalSection, and unlock calls LeaveCriticalSection. You could replace soProcessLock with TRTLCriticalSection...it's just a personal preference to not use Enter/Leave rather use Lock/Unlock.Harmsworth
OK, I actually used TCriticalSection, that seems the same thing your wrapper does but uses either Enter/Leave or Acquire/Release. Working great for me. Thanks!Burrow
Added a GitHub repo for this Thread source. Will introduce some fixes github.com/darianmiller/d5xlibHarmsworth
F
6

To elaborate on the original answer, (and on Smasher's rather short explanation), create a TEvent object. This is a synchronization object that's used for threads to wait on the right time to continue.

You can think of the event object as a traffic light that's either red or green. When you create it, it's not signaled. (Red) Make sure that both your thread and the code that your thread is waiting on have a reference to the event. Then instead of saying Self.Suspend;, say EventObject.WaitFor(TIMEOUT_VALUE_HERE);.

When the code that it's waiting on is finished running, instead of saying ThreadObject.Resume;, you write EventObject.SetEvent;. This turns the signal on (green light) and lets your thread continue.

EDIT: Just noticed an omission above. TEvent.WaitFor is a function, not a procedure. Be sure to check it's return type and react appropriately.

Fanniefannin answered 19/1, 2010 at 22:0 Comment(2)
The short length of the explanation in the original question is of course, the thing that makes me ask the new question. So I am trying to work out a full sample of an implementation. (Question above now shows sample code using a Win32 Event handle.)Dierolf
what is the type of EventObject?Braise
B
4

You could use an event (CreateEvent) and let the thread wait (WaitForObject) until the event is signaled (SetEvent). I know that this is a short answer, but you should be able to look these three commands up on MSDN or wherever you want. They should do the trick.

Bluepencil answered 19/1, 2010 at 21:47 Comment(1)
I had thought of a simple implementation like this, but I am worried about the shutdown problems, race conditions and deadlocks inherent in a cleanup of a thread which might be active or "suspended", and whether or not TThread.Terminated can be made to also signal reliably, using the same Event object, or perhaps using multiple objects and a WaitForMultiple... call.Dierolf
W
1

Your code uses a Windows event handle, it should better be using a TEvent from the SyncObjs unit, that way all the gory details will already be taken care of.

Also I don't understand the need for a waiting time - either your thread is blocked on the event or it isn't, there is no need for the wait operation to time out. If you do this to be able to shut the thread down - it's much better to use a second event and WaitForMultipleObjects() instead. For an example see this answer (a basic implementation of a background thread to copy files), you only need to remove the code dealing with file copying and add your own payload. You can easily implement your Start() and Stop() methods in terms of SetEvent() and ResetEvent(), and freeing the thread will properly shut it down.

Watanabe answered 20/1, 2010 at 15:22 Comment(7)
So I should have a shutdown event, plus a pause ('start/stop') event, and not do any busy-looping, even the tiniest bit?Dierolf
+1. I like the sample code there for the 'file copy' solution ('this answer').Dierolf
Incidentally the Gory Details are simply calls to CreateEvent,SetEvent,ResetEvent,and CloseHandle. Maybe its just better 'OOP'-ish code to use the already encapsulated Event class, but TEvent doesn't impress me much, and it adds a layer of abstraction that just might leak on me. (I don't mean a memory leak, I mean, a leaky abstraction, like TThread.Suspend, which is more of an explosive abstraction than a leaky one, IMHO.)Dierolf
Exactly, no busy loops. Make this a base class, and create a descendant class that implements Start() and Stop(). Another descendant class could simply have methods to manage a queue of tasks, and would (re)set the event in these methods accordingly, kind of what the file copy thread does. This two event solution is just the basic foundation that eliminates the busy loops and allows for clean thread shutdown.Watanabe
And I think the additional layer of abstraction is very nice here. If only because you will just FreeAndNil() all critical sections, events and mutexes, instead of calling the proper API function, and then only if the handle is valid. It's also to prevent possible problems down the road - FreeAndNil(fEvent); can probably be implemented for any OS that has threading, while CloseHandle(fEventHandle); ties you to the Windows API.Watanabe
I'm having problems, I clicked UpVote and it wrongly down-voted this answer from 1 to 0. And stupid Stack Overflow won't let me vote it back up.Dierolf
@Warren: That's the normal SO behaviour. You can change your vote only for a certain amount of time, and then only after edits to the answer. Do you want me to add or change something in the answer? ;-)Watanabe

© 2022 - 2024 — McMap. All rights reserved.