If I hook an event in a constructor, is there any possibility that the handler might be invoked by another thread before it's finished with the constructor?

e.g.:

private List<string> changes;

public MyClass(INotifyPropertyChanged observable) {
  observable.PropertyChanged += this.Handler;
  // Another thread changes a property at this point

  this.changes = new List<string>();
}

private void Handler(object sender, PropertyChangedEventArgs e) {
  this.changes.Add(e.PropertyName); // Breaks, because the list's not there yet
}

(Yes, I know it's trivial to avoid a problem in this example, I've got some more complex cases than this I'd like to make fully thread-safe)

I could probably just put a lock(obj) round both the event handler and the body of the constructor, but that feels clumsy and I suspect it's probably prone to deadlock somehow.

Is there a clean & reliable way of doing this?

2 Answers

0
Community On

How about using a thread-safe collection (like ConcurrentQueue) combined with the null conditional operator?

Thread-safe delegate invocation
Use the ?. operator to check if a delegate is non-null and invoke it in a thread-safe way (for example, when you raise an event).

class MyClass
{
    private ConcurrentQueue<string> changes;

    public MyClass(INotifyPropertyChanged observable)
    {
        observable.PropertyChanged += this.Handler;
        // Another thread changes a property at this point

        this.changes = new ConcurrentQueue<string>();
    }

    private void Handler(object sender, PropertyChangedEventArgs e)
    {
        this.changes?.Enqueue(e.PropertyName);
        // Nothing breaks, changes during construction are simply not recorded
    }
}
1
Dmytro Mukalov On

ECMA-335 doesn't obligate a CLI to provide guarantee that the initialization changes which are made in a constructor should be visible before the constructor completion:

It is explicitly not a requirement that a conforming implementation of the CLI guarantee that all state updates performed within a constructor be uniformly visible before the constructor completes (see there, section I.12.6.8).

So the brief answer: avoid a subscription of an instance event handlers inside a constructor because it implies an exposure of the instance to external consumers with no guarantee that the instance is ready for consumption.

In details: typically semantic of a constructor implies only state initialization that is getting internal data of an instance into consistent state (when all its invariants are true and it's ready for consumption by other objects). The mechanism of events in C# in essence is adaptation of observer pattern which implies number of interactions between its participants and making of subscription is one of those interactions and as any other interaction with other object it should be avoided in a constructor when the instance isn't guaranteed to be initialized. You correctly noticed the possible scenario when it can become a problem but even with applying of protection mechanisms like reordering or synchronization, it cannot be guaranteed 100% safe, because it may not be provided by a CLI implementation or even if provided there is still possibility of scenarios when a constructor fails to complete for a reason not dependent on code inside the constructor, for example because of ThreadAbortException.

Of course there can be some relaxed requirements to design dictated by some well-known constraints (for example you can be 100% sure that your event publisher is implemented in the way which excludes critical scenarios) but in general case I'd suggest to make separation of construction and subscription scenarios when there is separate method, which is part of public contract and which is purposed only for making subscriptions.