13

I want to do something like this in Java

  public void giveMoney(String userId, int money) {
    synchronized (userId) {

        Profile p = fetchProfileFromDB(userId);
        p.setMoney(p.getMoney() + userId);
        saveProfileToDB(p);

    }
   }

But of course, synchronizing on a string is not correct. What's a correct way to do something like this?

Bart van Heukelom
  • 43,244
  • 59
  • 186
  • 301

7 Answers7

10

If the set of user ids is limited, you can synchronize on an interned version of the String.

Either use String.intern() (which has a few drawbacks) or something like Guava Interners if you need a bit more control over the interning.

Joachim Sauer
  • 302,674
  • 57
  • 556
  • 614
5

In principle, you can synchronize on any object in Java. It's not in itself "not correct" to synchronize on a String object; it depends on what exactly you're doing.

But if userId is a local variable in a method, then this is not going to work. Each thread that executes the method with have its own copy of the variable (presumably referring to a different String object for each thread); synchronizing between threads ofcourse only works when you make multiple threads synchronize on the same object.

You'd have to make the object you're synchronizing on a member variable of the object that contains the method in which you have the synchronized block. If multiple threads are then calling the method on the same object, you'll achieve mutual exclusivity.

class Something {
    private Object lock = new Object();

    public void someMethod() {
        synchronized (lock) {
            // ...
        }
    }
}

You could also use explicit locks from the java.util.concurrent.locks package, that can give you more control if you need that:

class Something {
    private Lock lock = new ReentrantLock();

    public void someMethod() {
        lock.lock();
        try {
            // ...
        } finally {
            lock.unlock();
        }
    }
}

Especially if you want an exclusive lock for writing, but you don't want threads to have to wait for each other when reading, you might want to use a ReadWriteLock.

Jesper
  • 202,709
  • 46
  • 318
  • 350
  • This is exactly what I was referring to in my answer, although I wasn't as elegant. – mrkhrts Sep 26 '11 at 13:41
  • Yes, your second paragraph explains why I call it incorrect. – Bart van Heukelom Sep 26 '11 at 13:42
  • 1
    Regarding your expansion, yes, I could lock every time I update anybody's money, which would definitely work as a second-best solution. That doesn't allow me to update multiple users in parallel though. – Bart van Heukelom Sep 26 '11 at 13:44
  • If this is all about doing atomic database updates (fetch, update, store), then you should use database transactions and let the database handle the locking of rows. – Jesper Sep 26 '11 at 13:47
  • @Jesper It is, and I know, but the database doesn't currently support it and it's not mine. – Bart van Heukelom Sep 26 '11 at 13:56
  • @Jesper: Your comment is generally true, but does not solve @BartvanHeukelom problem: he needs to associate a unique user with a lock. The simple solution will be to maintain `String -> Object` map with user as a key, and any object as a lock or to use solution proposed by @irreputable. – dma_k Sep 26 '11 at 23:58
  • @dma_k I know, and the answer by Joachim that Bart accepted also works well. – Jesper Sep 27 '11 at 05:32
4

I guess there are a few options.

The easiest is that you could map a userId to a lock object in a threadsafe map. Others have mentioned interning but I don't think that's a viable option.

However, the more common option would be to synchronize on p (the Profile). This is appropriate if getProfile() is threadsafe, and by its name I would suspect it might be.

Mark Peters
  • 80,126
  • 17
  • 159
  • 190
3

Theoretically speaking, since interned objects can be GC-ed, it's possible to synchronized on different objects (of the same value) at different times. Mutual exclusivity is still guaranteed, since it's not possible to synchronized on different objects at the same time.

However, if we synchronized on different objects, the happens-before relation is at doubt. We have to examine the implementation to find out. And since it involves GC, which Java Memory Model does not address, the reasoning can be quite difficult.

That's a theoretical objection; practically I don't think it'll cause any problem.

Still, there can be simple, direct, and theoretically correct solution to your problem. For example Simple Java name based locks?

Community
  • 1
  • 1
irreputable
  • 44,725
  • 9
  • 65
  • 93
  • I don't mind if the interned IDs are GC'd, as long as the mutual exclusivity is guaranteed. In fact I want them to be GC'd, because there'll be many different IDs. Thanks for the link, it's also a good option. – Bart van Heukelom Sep 27 '11 at 08:50
1

You can use a proxy object for the string.

Object userIdMutex = new Object();

synchronized (userIdMutex) {
    Profile p = getProfile(userId);
    p.setMoney(p.getMoney() + p);
    saveProfile(p);
}

Use this mutex whenever you access userId.

mrkhrts
  • 829
  • 7
  • 17
0

Based on your example, I assume you want to obtain a lock to a profile class, change it, and then release the lock. Synchronization is not exactly what you need in my opinion. You need a class that manages those records and lets you lock and unlock a record when changes need to be made to it, aka source control style.

Check this out: Lock class Java 5

Ravi Wallau
  • 10,416
  • 2
  • 25
  • 34
-4

What about this:

String userId = ...;
Object userIdLock = new Object();
synchronized (userIdLock) {
    Profile p = getProfile(userId);
    p.setMoney(p.getMoney() + p);
    saveProfile(p);
}

It's simple and above all obvious.

bugs_
  • 3,544
  • 4
  • 34
  • 39