开发者

Should I use ManualResetEvent as a lock object?

开发者 https://www.devze.com 2023-02-17 21:58 出处:网络
The method below should return true for the first call, and false for any other call. Is there any problem with it? Is it safe to use the reset event for locking?

The method below should return true for the first call, and false for any other call.

Is there any problem with it? Is it safe to use the reset event for locking?

private ManualResetEvent _resetEvent开发者_C百科 = new ManualResetEvent(false);

public bool AmIFirst()
{
    lock (_resetEvent)
    {
        bool first = !_resetEvent.WaitOne(0);
        if (first)
            _resetEvent.Set();

        return first;
    }
}

Edit: I made some changes after reviewing you're remarks. I was stuck on ManualResetEvent due to former design idea. I actually don't need it at all.

class ActionSynchronizer
{
    private Timer _expirationTimer;
    private object _locker = new object();
    private bool _executionRequired = true;

    private SomeDelegate _onExpired = delegate { };

    public ActionSynchronizer(SomeDelegate onExpired)
    {
        _onExpired = onExpired;
        expirationTimer = new Timer(OnExpired, null, 30000, Timeout.Infinite);
    }

    public bool IsExecutionRequired()
    {
        if (!_executionRequired)
            return false;

        lock (_locker)
        {
            if (_executionRequired)
            {
                _executionRequired = false;
                return true;
            }

            return false;
        }
    }

    private void OnExpired(object state)
    {
        if (_executionRequired)
        {
            lock (_locker)
            {
                if (_executionRequired)
                {
                    _executionRequired = false;
                    // http://stackoverflow.com/questions/1712741/why-does-asynchronous-delegate-method-require-calling-endinvoke/1712747#1712747
                    _onExpired.BeginInvoke(_originalAction, EndInvoke, null);
                }
            }
        }
    }
}

// ...
{
    if (_action.Sync.IsExecutionRequired())
        _action.Invoke();
}


I would go a different route here...

private int counter;
...
if(Interlocked.Increment(ref counter) == 1)
{
     // yes, I'm first
}

Thread safe, no locks. Or if you are worried about wrapping around Int32:

if(Interlocked.CompareExchange(ref counter, 1, 0) == 0)
{
     // yes, I'm first
}   


Nowadays, I only ever lock() on a simple System.Object object which I've created just for locking with.

I definitely wouldn't lock() on something like an Event, not because it wouldn't work, but because I think it's potentially rather confusing to be using lock() on an object which it is itself (though completely separately) associated with kernel locking type operations.

I'm not clear what you're actually doing here, but it looks rather like something which a named Mutex might do better.


I think it's better to use lock() for this on an object.

Also, you can prevent excess thread locking by using a "double-checked locking"

e.g.

private object _protection = new object();
private bool _firstTime = true;

public bool AmIFirst()
{
    if (!_firstTime)
        return false;

    lock (_protection)
    {
        if (!_firstTime)
            return false;

        _firstTime = false;
        return true;
    }
}

Note... - there's some interesting comments on double-checked locking - Double-checked locking in .NET - I'm still reading up on this!


Another note... its not clear from the code snippet you posted, but if you are looking to implement a global singleton then solution 4 on http://www.yoda.arachsys.com/csharp/singleton.html is a good place to start


The only thing you need to make sure is that the same object you lock on is accessible to all instances of the code that needs synchronizing. Other than that, no problem.

0

精彩评论

暂无评论...
验证码 换一张
取 消