Here's some code I saw once. Can you see what's wrong with it?
[updated]
public class ResourceManager1
{
private final String mutex = "";
Object resource = null;
public Object getResource()
{
synchronized (mutex)
{
if (resource == null)
{
resource = new Object();
}
}
return resource;
}
}
public class ResourceManager2
{
private final String mutex = "";
Object resource = null;
public Object getResource()
{
synchronized (mutex)
{
if (resource == null)
{
resource = new Object();
}
}
return resource;
}
}
Best Solution
Never synchronize on strings, particularly string literals which are interned. You've basically just got a single lock.
In general, never synchronize on any reference that might be visible outside your class (including "this") unless the purpose of the external visibility is precisely for locking purposes. I usually use a
private final
variable created solely for the purpose of locking.