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
281 views
in Technique[技术] by (71.8m points)

c# - Locking by string. Is this safe/sane?

I need to lock a section of code by string. Of course the following code is hideously unsafe:

lock("http://someurl")
{
    //bla
}

So I've been cooking up an alternative. I'm not normally one to post large bodies of code here, but when it comes to concurrent programming, I'm a little apprehensive about making my own synchronization scheme, so I'm submitting my code to ask if it's sane to do it in this way or whether there's a more straightforward approach.

public class StringLock
{
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>();
    private readonly object keyLocksLock = new object();

    public void LockOperation(string url, Action action)
    {
        LockObject obj;
        lock (keyLocksLock)
        {
            if (!keyLocks.TryGetValue(url,
                                      out obj))
            {
                keyLocks[url] = obj = new LockObject();
            }
            obj.Withdraw();
        }
        Monitor.Enter(obj);
        try
        {
            action();
        }
        finally
        {
            lock (keyLocksLock)
            {
                if (obj.Return())
                {
                    keyLocks.Remove(url);
                }
                Monitor.Exit(obj);
            }
        }
    }

    private class LockObject
    {
        private int leaseCount;

        public void Withdraw()
        {
            Interlocked.Increment(ref leaseCount);
        }

        public bool Return()
        {
            return Interlocked.Decrement(ref leaseCount) == 0;
        }
    }
}

I would use it like this:

StringLock.LockOperation("http://someurl",()=>{
    //bla
});

Good to go, or crash and burn?

EDIT

For posterity, here's my working code. Thanks for all the suggestions:

public class StringLock
{
    private readonly Dictionary<string, LockObject> keyLocks = new Dictionary<string, LockObject>();
    private readonly object keyLocksLock = new object();

    public IDisposable AcquireLock(string key)
    {
        LockObject obj;
        lock (keyLocksLock)
        {
            if (!keyLocks.TryGetValue(key,
                                      out obj))
            {
                keyLocks[key] = obj = new LockObject(key);
            }
            obj.Withdraw();
        }
        Monitor.Enter(obj);
        return new DisposableToken(this,
                                   obj);
    }

    private void ReturnLock(DisposableToken disposableLock)
    {
        var obj = disposableLock.LockObject;
        lock (keyLocksLock)
        {
            if (obj.Return())
            {
                keyLocks.Remove(obj.Key);
            }
            Monitor.Exit(obj);
        }
    }

    private class DisposableToken : IDisposable
    {
        private readonly LockObject lockObject;
        private readonly StringLock stringLock;
        private bool disposed;

        public DisposableToken(StringLock stringLock, LockObject lockObject)
        {
            this.stringLock = stringLock;
            this.lockObject = lockObject;
        }

        public LockObject LockObject
        {
            get
            {
                return lockObject;
            }
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        ~DisposableToken()
        {
            Dispose(false);
        }

        private void Dispose(bool disposing)
        {
            if (disposing && !disposed)
            {
                stringLock.ReturnLock(this);
                disposed = true;
            }
        }
    }

    private class LockObject
    {
        private readonly string key;
        private int leaseCount;

        public LockObject(string key)
        {
            this.key = key;
        }

        public string Key
        {
            get
            {
                return key;
            }
        }

        public void Withdraw()
        {
            Interlocked.Increment(ref leaseCount);
        }

        public bool Return()
        {
            return Interlocked.Decrement(ref leaseCount) == 0;
        }
    }
}

Used as follows:

var stringLock=new StringLock();
//...
using(stringLock.AcquireLock(someKey))
{
    //bla
}
See Question&Answers more detail:os

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

1 Reply

0 votes
by (71.8m points)

Locking by an arbitrary string instance would be a bad idea, because Monitor.Lock locks the instance. If you had two different string instances with the same content, that would be two independent locks, which you don't want. So you're right to be concerned about arbitrary strings.

However, .NET already has a built-in mechanism to return the "canonical instance" of a given string's content: String.Intern. If you pass it two different string instances with the same content, you will get back the same result instance both times.

lock (string.Intern(url)) {
    ...
}

This is simpler; there's less code for you to test, because you'd be relying on what's already in the Framework (which, presumably, already works).


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

...