Is it OK to use a string as a lock object?

Solution 1:

Locking on strings is discouraged, the main reason is that (because of string-interning) some other code could lock on the same string instance without you knowing this. Creating a potential for deadlock situations.

Now this is probably a far fetched scenario in most concrete situations. It's more a general rule for libraries.

But on the other hand, what is the perceived benefit of strings?

So, point for point:

Are there any problems with this approach?

Yes, but mostly theoretical.

Is it OK to lock on a string object in this way, and are there any thread safety issues in using the HashSet?

The HashSet<> is not involved in the thread-safety as long as the threads only read concurrently.

Is it better to, for example, create a Dictionary that creates a new lock object for each string instance?

Yes. Just to be on the safe side. In a large system the main aim for avoiding deadlock is to keep the lock-objects as local and private as possible. Only a limited amount of code should be able to access them.

Solution 2:

I'd say it's a really bad idea, personally. That isn't what strings are for.

(Personally I dislike the fact that every object has a monitor in the first place, but that's a slightly different concern.)

If you want an object which represents a lock which can be shared between different instances, why not create a specific type for that? You can given the lock a name easily enough for diagnostic purposes, but locking is really not the purpose of a string. Something like this:

public sealed class Lock
{
    private readonly string name;

    public string Name { get { return name; } }

    public Lock(string name)
    {
        if (name == null)
        {
            throw new ArgumentNullException("name");
        }
        this.name = name;
    }
}

Given the way that strings are sometimes interned and sometimes not (in a way which can occasionally be difficult to discern by simple inspection), you could easily end up with accidentally shared locks where you didn't intend them.