Is a method pointer assignment thread safe?

Question

Example:

Assume, I would have the following thread (please, don't take into account what's used in this example's thread context execution method, it's just for explanation):

type
  TSampleThread = class(TThread)
  private
    FOnNotify: TNotifyEvent;
  protected
    procedure Execute; override;
  public
    property OnNotify: TNotifyEvent read FOnNotify write FOnNotify;
  end;

implementation

procedure TSampleThread.Execute;
begin
  while not Terminated do
  begin
    if Assigned(FOnNotify) then
      FOnNotify(Self); // <- this method can be called anytime
  end;
end;

Then assume, I'd like to change the method of the OnNotify event from the main thread at any time I need. This main thread implements the event handler method as the ThreadNotify method here:

type
  TForm1 = class(TForm)
    Button1: TButton;
    Button2: TButton;
    procedure Button1Click(Sender: TObject);
    procedure Button2Click(Sender: TObject);
  private
    FSampleThread: TSampleThread;
    procedure ThreadNotify(Sender: TObject);
  end;

implementation

procedure TForm1.ThreadNotify(Sender: TObject);
begin
  // do something; unimportant for this example
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  FSampleThread.OnNotify := nil; // <- can this be changed anytime ?
end;

procedure TForm1.Button2Click(Sender: TObject);
begin
  FSampleThread.OnNotify := ThreadNotify; // <- can this be changed anytime ?
end;

Question:

Is it safe to change a method, which can be called from a worker thread from another thread's context at any time ? Is it safe to do what is shown in the above example ?

I'm not precisely sure, if that's absolutely safe, at least since method pointer is actually a pair of pointers and I don't know if I can take it as an atomic operation.

Answer 1

No, it's not thread safe because that operation will never be "atomic". The TNotifyEvent is made up of two pointers, and those pointers will never be both assigned at the same time: one will be assigned, then the other will be assigned.

The 32 bit Assembler generated for a TNotifyEvent assignment is composed of two distinct assembler instructions, something like this:

MOV [$00000000], Object
MOV [$00000004], MethodPointer

If it were a single pointer then you'd have some options, since that operation is atomic: the options you have depend on how strong the memory model of the CPU is:

• If the CPU supports the "sequential consistency" model, then any read that happens after you write the memory will see the new value, guaranteed. If that's the case you can simply write your value, no need for Memory Barriers or the use of Interlocked methods.
• If the CPU is more relaxed about re-ordering stores and loads, then you need a "memory barrier". If that's the case, the easiest solution is to use InterlockedExchangePointer

Unfortunately I do not know how strong the memory model of current Intel CPUs is. There's some circumstantial evidence that suggest some re-ordering may take place, those the use of Interlocked would be recommend, but I haven't seen a definitive statement by Intel that says one or the other.

Evidence:

• Modern CPU's use "prefetch" - that automatically implies some level of load/store reordering.
• SSE introduced specific instructions for dealing with CPU cache.

Answer 2

Beside more than register size, there are two operations involved. A check and later executed. To minimized, create a local var and use it. But anyway, this is still not 100% thread safe

var
  LNotify: TNotifyEvent;
begin
  ...
  LNotify := FOnNotify;
  if Assigned(LNotify) then
    LNotify(Self);
end;