Is it safe to remove and reassign events this way? If not, why?

Question

A.Event1 := nil;
A.Event2 := nil;
try
  ...
finally
  A.Event1 := MyEvent1;
  A.Event2 := MyEvent2;
end;

Can something go wrong with it?

EDIT:

I've accepted Barry's answer because it answered exactly what I asked, but Vegar's answer is also correct depending on the scenario, sadly I can't accept both.

Answer 1

It entirely depends on what happens in the bit of code marked '...'. If it e.g. starts up a background thread and tries to invoke Event1 or Event2 after execution has continued into the finally block, you may get unexpected results.

If the code is entirely single-threaded, then yes, neither Event1 nor Event2 should be triggered while the code is between the try and finally.

However, that does assume that Event1 and Event2 properties (all Delphi events are properties of a method pointer type) do not do unusual things in their setters. A pathologically malicious event setter could squirrel away a method pointer, and still be able to invoke it, even after you later call the setter with 'nil' as the value.

But that would be highly unusual behaviour.

Answer 2

This sounds like an event-nightmare I have seen before :-)

Instead of removing the events, I usually set a flag that I check in the event. I often use a integer rather than boolean so that the same flag can be set multiple times in one processing.

Something like this:

procedure TMyObject.Traverse;
begin
  inc(FTraverseFlag);
  try
    ...
  finally
    dec(FTracerseFlag);
  end;
end;

procedure TMyObject.OnBefore( ... );
begin
  if FTraverseFlag > 0 then 
    exit;
  ...
end;

I guess this easily could be made thread-safe to solve Barrys concerns.

Answer 3

As Barry said, the only real concern is with multithreaded concerns - other than that is perfectly normal. As VCL events setters just assign the event, nothing need to be worried.

Answer 4

If it is multithreaded you need to make operation atomic. Disabling several event handlers opens possibility that they are running at the moment then they are being disabled. Or Event2 starts when Event1 is being set to nil. All kinds of malfunctions may happen if these events called often. Possible solution to use semaphores instead. But then you may need to add extra code to event handlers as well.

Any way setting event handler temporarily to nil looks like a bad design. It will be incomprehensible very soon and hard to make do what it have to do any way. So you’d better spend some time and develop something more sophisticated.