CODEWITHSUNDEEP
multithreadingdelphithread-safety
Martin Reiner
Martin Reiner

Reputation: 2197

Is it safe to set the boolean value in thread from another one?

I'm wondering if the following (pseudo) code is safe to use. I know about Terminated flag but I need to set some sort of cancel flag at recursive search operation from the main thread and keep the worker thread running. I will check there also the Terminated property, what is missing in this pseudo code.

type
  TMyThread = class(TThread)
  private
    FCancel: Boolean;
    procedure RecursiveSearch(const ItemID: Integer);
  protected
    procedure Execute; override;
  public
    procedure Cancel;
end;

procedure TMyThread.Cancel;
begin
  FCancel := True;
end;

procedure TMyThread.Execute;
begin
  RecursiveSearch(0);
end;

procedure TMyThread.RecursiveSearch(const ItemID: Integer);
begin
  if not FCancel then
    RecursiveSearch(ItemID);  
end;

procedure TMainForm.ButtonCancelClick(Sender: TObject);
begin
  MyThread.Cancel;
end;

Is it safe to set the boolean property FCancel inside of the thread this way ? Wouldn't this collide with reading of this flag in the RecursiveSearch procedure while the button in the main form (main thread) is pressed ? Or will I have to add e.g. critical section for reading and writing of this value ?

Thanks a lot

Upvotes: 16

Views: 13912

Answers (4)

Dean Roberson
Dean Roberson

Reputation: 189

I agree that writing a boolean from one thread and reading from another is thread safe. However, be careful with incrementing - this is not atomic and may cause a decidedly non-benigh race condition in your code depending on the implementation. Increment/Decrement normally turns into three seperate machine instructions - load/inc/store.

This is what the InterlockedIncrement, InterlockedDecrement, and InterlockedExchange Win32 API calls are for - to enable 32-bit increment, decrement, and loads to occur atomically without a seperate synchronization object.

Upvotes: 4

David Heffernan
David Heffernan

Reputation: 613003

It's perfectly safe to do this. The reading thread will always read either true or false. There will be no tearing because a Boolean is just a single byte. In fact the same is true for an aligned 32 bit value in a 32 bit process, i.e. Integer.

This is what is known as a benign race. There is a race condition on the boolean variable since one thread reads whilst another thread writes, without synchronisation. But the logic of this program is not adversely affected by the race. In more complex scenarios, such a race could be harmful and then synchronisation would be needed.

Upvotes: 27

user497849
user497849

Reputation:

Yes it is safe, you need to use Critical Sections only when you are reading or writing from/to another thread, within same thread it is safe.

BTW. the way you have RecursiveSearch method defined, if (FCancel = False) then you'll get an Stack overflow (:

Upvotes: 1

dthorpe
dthorpe

Reputation: 36082

Writing to a boolean field from different threads is thread safe - meaning, the write operation is atomic. No observer of the field will ever see a "partial value" as the value is being written to the field. With larger data types, partial writes are a real possibility because multiple CPU instructions are required to write the value to the field.

So, the actual write of the boolean is not a thread safety issue. However, how observers are using that boolean field may be a thread safety issue. In your example, the only visible observer is the RecursiveSearch function, and its use of the FCancel value is pretty simple and harmless. The observer of the FCancel state does not change the FCancel state, so this is a straight / acyclic producer-consumer type dependency.

If instead the code was using the boolean field to determine whether a one-time operation needed to be done, simple reads and writes to the boolean field will not be enough because the observer of the boolean field also needs to modify the field (to mark that the one-time operation has been done). That's a read-modify-write cycle, and that is not safe when two ore more threads perform the same steps at just the right time. In that situation, you can put a mutex lock around the one-time operation (and boolean field check and update), or you can use InterlockedExchange to update and test the boolean field without a mutex. You could also move the one-time operation into a static type constructor and not have to maintain any locks yourself (though .NET may use locks behind the scenes for this).

Upvotes: 10

Related Questions