How do I conditionally lock a method based on its parameter in Java?

I have the following method which runs 'SubJobs' for a given JobModel parameter:

public void runSubJobs(JobModel jobModel) {
     LOGGER.log("Start executing SubJobs for " + jobModel.getId());
     for (SubJobModel subJobModel : jobModel.getSubJobs()) {
           run(subJobModel);
     }
     LOGGER.log("Finished executing SubJobs for " + jobModel.getId());
}

In the current state the method can run parallel on different threads but I want to restrict it to the following condition:

runSubJobs() should only run in parallel for different JobModel objects, no parallel execution for the same JobModel parameter

What's the best practice to solve this? Can I just surround the content of the method with synchronized (jobModel) {} like this?:

public void runSubJobs(JobModel jobModel) {
  synchronized (jobModel) {
     LOGGER.log("Start executing SubJobs for " + jobModel.getId());
     for (SubJobModel subJobModel : jobModel.getSubJobs()) {
           run(subJobModel);
     }
     LOGGER.log("Finished executing SubJobs for " + jobModel.getId());
  }
}

synchronized (x) {} works like this:

• dereference x. It's not about the variable, it's the object it is pointing at. Given Object y = new Object(); Object x = y;, synchronized (y) {} and synchronized (x) {} would function identically, as x and y are pointing at the same object. Given that it dereferences, if x is null, that would result in a NullPointerException.

• All objects have a hidden extra field; it looks like: private Thread owner = null;.

• synchronized (x) { tries to run if (x.owner == null) { x.owner = Thread.currentThread(); } else { just keep trying but without wasting all that much CPU cycles checking } - and, of course, atomically. Also, threads are 're-entrant'. synchronized (x) { when the object x is pointing at is already marked as 'this thread owns it' works fine.

• Exiting that sync block in any way (run to the }, return; or break; out of it, or even throw an exception that leads out of it), will run x.owner = null; thus letting any other thread that is 'blocked' waiting to become the owner of this object get on with it.

• Resolution of conflict is indeterminate and comes with no guarantee of fairness. In other words, if 5 threads are all blocking because they are trying to become the owner of the same object and that object currently is owned by another thread, and that thread exits the sync block - then the JVM makes no promises about who 'wins'. In particular, the thread that has been waiting the longest has absolutely no advantage. The JVM does not guarantee that it rolls a fair die either. It just doesn't tell you how it decides, and in practice it depends on the system, OS, phase of the moon, and which song you're playing at the time as far as you know. If your software doesn't work correctly unless 'who wins the fight' goes a certain way, your software is buggy in an untestable way. Which is bad.

Hence: Yeah sure that should work fine. Generally you want locks to be well documented: You should document in the javadoc of JobModel that locking on it would also stop any sub job processing and is therefore strongly advised against. If this code is in the same package as JobModel, I recommend you instead make a dedicated lock object and make it package private:

public class JobModel {
  final Object subJobRunnerLock = new Object[0];
}

public class SubJobRunner {
  public void runSubJobs(JobModel m) {
    synchronized (m.subJobRunnerLock) {
      .. process em here ..
    }
  }
}

This avoids having other code outside of your direct control acquire locks and thus completely asplode your code by accident. You very very rarely want to lock on public anything.

NB: new Object[0] in case you want JobModel to be serializable; it's just a simple to create tiny object that, unlike new Object();, is serializable. If serializability doesn't matter, new Object() is just as easy.

Note that this is fairly low-level stuff. The java.util.concurrent package has all sorts of useful utilities that tend to work a lot better. For example, there's a ReadWriteLock which works a lot like these but has the additional ability to have a 'one writer many readers' setup: Any amount of readers can acquire the lock and run simultaneously, but only one writer ever can (and readers block the writer, and the writer blocks the readers). A common job that is quite difficult to manually write and manage with synchronized, wait(), and notifyAll() (which are the concurrency management primitives baked into the JVM).

One serious issue with managing multi-core stuff is that it is by and large utterly untestable - the JVM has all sorts of room to act as it wants, but, on many systems it reliably acts in one way. Meaning, it is very simple to write code that works perfectly all day today. But it never works on the computer of your friend, and next week every few hours it randomly doesn't work. Hence, 'experimentation', ordinarily a great way to learn to program, works out horribly here. All the more reason to use stuff from java.util.concurrent!