多线程环境中的事件

Question

我正在尝试构建一个系统，用户可以通过该系统构建一个小测试程序，而无需知道如何编码。为此，我以这种方式设计了系统，即有一个过程，其中可以包含其他过程或步骤。这些步骤可以包含命令。该过程包含以何种顺序发生的逻辑。这些步骤包含它们接下来要连接到哪个步骤的信息。

步骤和命令由Execute 调用，如果它们完成，则调用OnDone，这可能直接发生（例如IncreaseCommand）或在一段时间后（WaitCommand 或任何其他与连接的硬件；两者都在不同的线程上）。此外，它们可以通过超时或用户停止。

只要没有超时，一切正常。如果超时，我尝试通过锁定使代码线程安全。此外，当超时停止一个命令（例如WaitCommand）时，也会出现这些陷阱，该命令正在同时完成其工作。所以有一个线程从过程通过步骤到命令，发出停止信号，另一个线程从命令通过步骤到过程发出信号完成。

我添加了一些代码 sn-ps，我已经剥离了大部分 dispose 代码和其他内部内容，这似乎与问题无关。

public sealed class Procedure : IStep, IStoppable
{
    public event EventHandler Done;
    public event EventHandler Stopped;
    public event EventHandler TimedOut;
    public void Run()
    {
        if (!IsRunning)
        {
            CheckStartTimer();
            Start(First);
        }
    }
    private void CheckStartTimer()
    {
        isTimerUnlinked = false;
        timer.Elapsed += OnTimedOut;
        timer.IntervalInMilliseconds = (int)Timeout.TotalMilliseconds;
        timer.Start();
    }
    private void OnTimedOut(object sender, EventArgs e)
    {
        if (isTimerUnlinked)
            return;
        stopFromTimeout = true;
        Stop();
    }
    private void Start(IStep step)
    {
        isStopped = false;
        isStopping = false;
        Active = step;
        LinkActive();
        active.Run();
    }
    private void LinkActive()
    {
        active.Done += OnActiveFinished;
        if (active is Procedure proc)
            proc.TimedOut += OnActiveFinished;
    }
    private void OnActiveFinished(object sender, EventArgs e)
    {
        UnlinkActive();
        lock (myLock)
        {
            if (isStopped)
                return;
            if (stopFromTimeout)
            {
                OnStopped();
                return;
            }
        }
        var successor = active.ActiveSuccessor;
        if (successor == null)
            OnDone();
        else if (isStopping || timeoutPending || stopFromTimeout)
            OnStopped();
        else
            Start(successor);
    }
    public void Stop()
    {
        if (isStopping)
            return;
        isStopping = true;
        StopTimer();
        if (active is IStoppable stoppable)
        {
            stoppable.Stopped += stoppable_Stopped;
            stoppable.Stop();
        }
        else
            OnStopped();
    }
    private void stoppable_Stopped(object sender, EventArgs e)
    {
        var stoppable = sender as IStoppable;
        stoppable.Stopped -= stoppable_Stopped;
        OnStopped();
    }
    private void OnStopped()
    {
        isStopping = false;
        lock (myLock)
        {
            isStopped = true;
        }
        UnlinkActive();
        lock (myLock)
        {
            Active = null;
        }
        if (stopFromTimeout || timeoutPending)
        {
            stopFromTimeout = false;
            timeoutPending = false;
            CleanUp();
            TimedOut?.Invoke(this, EventArgs.Empty);
        }
        else
            Stopped?.Invoke(this, EventArgs.Empty);
    }
    private void UnlinkActive()
    {
        if (stopFromTimeout && !isStopped)
            return;
        lock (myLock)
        {
            if (active == null)
                return;
            active.Done -= OnActiveFinished;
            var step = active as IStep;
            if (step is Procedure proc)
                proc.TimedOut -= OnActiveFinished;
        }
    }
    private void OnDone()
    {
        CleanUp();
        Done?.Invoke(this, EventArgs.Empty);
    }
    private void CleanUp()
    {
        Reset();
        SetActiveSuccessor();
    }
    private void Reset()
    {
        Active = null;
        stopFromTimeout = false;
        timeoutPending = false;
        StopTimer();
    }
    private void StopTimer()
    {
        if (timer == null)
            return;
        isTimerUnlinked = true;
        timer.Elapsed -= OnTimedOut;
        timer.Stop();
    }
    private void SetActiveSuccessor()
    {
        ActiveSuccessor = links[(int)Successor.Simple_If];
    }
}

internal sealed class CommandStep : IStep, IStoppable
{
    public event EventHandler Done;
    public event EventHandler Started;
    public event EventHandler Stopped;
    public CommandStep(ICommand command)
    {
        this.command = command;
    }
    public void Run()
    {
        lock (myLock)
        {
            stopCalled = false;
            if (cookie != null && !cookie.Signalled)
                throw new InvalidOperationException(ToString() + " is already active.");
            cookie = new CommandStepCookie();
        }
        command.Done += OnExit;
        unlinked = false;
        if (stopCalled)
            return;
        command.Execute();
    }
    public void Stop()
    {
        stopCalled = true;
        if (command is IStoppable stoppable)
            stoppable.Stop();
        else
            OnExit(null, new CommandEventArgs(ExitReason.Stopped));
    }
    private void OnExit(object sender, CommandEventArgs e)
    {
        (sender as ICommand).Done -= OnExit;
        lock (myLock)
        {
            if (cookie.Signalled)
                return;
            cookie.ExitReason = stopCalled ? ExitReason.Stopped : e.ExitReason;
            switch (cookie.ExitReason)
            {
                case ExitReason.Done:
                default:
                    if (unlinked)
                        return;
                    Unlink();
                    ActiveSuccessor = links[(int)Successor.Simple_If];
                    break;
                case ExitReason.Stopped:
                    Unlink();
                    break;
                case ExitReason.Error:
                    throw new NotImplementedException();
            }
            cookie.Signalled = true;
        }
        if (cookie.ExitReason.HasValue)
        {
            active = false;
            if (cookie.ExitReason == ExitReason.Done)
                Done?.Invoke(this, EventArgs.Empty);
            else if (cookie.ExitReason == ExitReason.Stopped)
                stopCalled = false;
                Stopped?.Invoke(this, EventArgs.Empty);
        }
    }
    private void Unlink()
    {
        if (command != null)
            command.Done -= OnExit;
        unlinked = true;
    }
}

internal sealed class WaitCommand : ICommand, IStoppable
{
    public event EventHandler<CommandEventArgs> Done;
    public event EventHandler Stopped;
    internal WaitCommand(ITimer timer)
    {
        this.timer = timer;
        timer.AutoRestart = false;
        TimeSpan = TimeSpan.FromMinutes(1);
    }
    public void Execute()
    {
        lock (myLock)
        {
            cookie = new WaitCommandCookie(
                e => Done?.Invoke(this, new CommandEventArgs(e)));
            timer.IntervalInMilliseconds = (int)TimeSpan.TotalMilliseconds;
            timer.Elapsed += OnElapsed;
        }
        timer.Start();
    }
    private void OnElapsed(object sender, EventArgs e)
    {
        OnExit(ExitReason.Done);
    }
    public void Stop()
    {
        if (cookie == null)
        {
            Done?.Invoke(this, new CommandEventArgs(ExitReason.Stopped));
            return;
        }
        cookie.Stopping = true;
        lock (myLock)
        {
            StopTimer();
        }
        OnExit(ExitReason.Stopped);
    }
    private void OnExit(ExitReason exitReason)
    {
        if (cookie == null)
            return;
        lock (myLock)
        {
            if (cookie.Signalled)
                return;
            Unlink();
            if (cookie.Stopping && exitReason != ExitReason.Stopped)
                return;
            cookie.Stopping = false;
        }
        cookie.Signal(exitReason);
        cookie = null;
    }
    private void StopTimer()
    {
        Unlink();
        timer.Stop();
    }
    private void Unlink()
    {
        timer.Elapsed -= OnElapsed;
    }
}

我一直在某些地方测试停止是否正在进行并试图拦截完成，以便在停止后不执行并造成任何麻烦。这种方式似乎并不完全防水，尽管它似乎暂时有效。有没有办法通过设计来提供这种安全性？我可能这样做完全错了吗？

Answer 1

您的共享状态并未始终受到并发访问的保护。以isStopped 字段为例：

private void OnStopped()
{
    isStopping = false;
    lock (myLock)
    {
        isStopped = true;
    }
    //...

private void Start(IStep step)
{
    isStopped = false;
    //...

首先它受到保护，其次它没有。您可以选择在任何地方保护它，也可以在任何地方保护它。没有回旋的余地。部分保护它与完全不保护它一样好。

附带说明，不建议在持有锁的同时调用事件处理程序。事件处理程序可能包含长时间运行的代码，或者调用可能受其他锁保护的任意代码，从而打开死锁的可能性。关于锁定的一般建议是尽快释放它。持有锁的时间越长，线程之间的争用就越多。所以例如在方法OnActiveFinished:

lock (myLock)
{
    if (isStopped)
        return;
    if (stopFromTimeout)
    {
        OnStopped();
        return;
    }
}

您在持有锁的同时拨打OnStopped。在OnStopped 中调用处理程序：

Stopped?.Invoke(this, EventArgs.Empty);

正确的方法是释放锁后调用OnStopped。使用局部变量来存储有关调用它的信息：

var localInvokeOnStopped = false;
lock (myLock)
{
    if (isStopped)
        return;
    if (stopFromTimeout)
    {
        localInvokeOnStopped = true;
    }
}
if (localInvokeOnStopped)
{
    OnStopped();
    return;
}

最后一个建议，避免在同一个锁上递归锁定。如果你这样做，lock 语句不会抱怨（因为底层的Monitor 类允许重入），但它会使你的程序更难理解和维护。