Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Welcome To Ask or Share your Answers For Others

Categories

0 votes
1.1k views
in Technique[技术] by (71.8m points)

multithreading - C#: Thread-safe events

Is the implementation below thread-safe? If not what am I missing? Should I have the volatile keywords somewhere? Or a lock somewhere in the OnProcessingCompleted method? If so, where?

public abstract class ProcessBase : IProcess
{
    private readonly object completedEventLock = new object();

    private event EventHandler<ProcessCompletedEventArgs> ProcessCompleted;

    event EventHandler<ProcessCompletedEventArgs> IProcess.ProcessCompleted
    {
        add
        {
            lock (completedEventLock)
                ProcessCompleted += value;
        }
        remove
        {
            lock (completedEventLock)
                ProcessCompleted -= value;
        }
    }

    protected void OnProcessingCompleted(ProcessCompletedEventArgs e)
    {
        EventHandler<ProcessCompletedEventArgs> handler = ProcessCompleted;
        if (handler != null)
            handler(this, e);
    }
}

Note: The reason why I have private event and explicit interface stuff, is because it is an abstract base class. And the classes that inherit from it shouldn't do anything with that event directly. Added the class wrapper so that it is more clear =)

See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome To Ask or Share your Answers For Others

1 Reply

0 votes
by (71.8m points)

You need to lock when you fetch the handler too, otherwise you may not have the latest value:

protected void OnProcessingCompleted(ProcessCompletedEventArgs e)
{
    EventHandler<ProcessCompletedEventArgs> handler;
    lock (completedEventLock) 
    {
        handler = ProcessCompleted;
    }
    if (handler != null)
        handler(this, e);
}

Note that this doesn't prevent a race condition where we've decided we're going to execute a set of handlers and then one handler unsubscribed. It will still be called, because we've fetched the multicast delegate containing it into the handler variable.

There's not a lot you can do about this, other than making the handler itself aware that it shouldn't be called any more.

It's arguably better to just not try to make the events thread-safe - specify that the subscription should only change in the thread which will raise the event.


与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
OGeek|极客中国-欢迎来到极客的世界,一个免费开放的程序员编程交流平台!开放,进步,分享!让技术改变生活,让极客改变未来! Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Click Here to Ask a Question

...