Configurable synchronization approach in Java

Question

I am interested in the community opinion about the following approach for the synchronization. Generally it is based on the idea to have a configurable way to apply locking on some logic parts.

Any comments and review is appreciated :) .

lock.properties keeps configuration:

foo=true
bar=false

SynFactory.java produces locks:

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
public class SyncFactory {
 private static HashMap<String, Object> locks = null;
 public static synchronized Object getLock(String key) {
 init();
 return locks.containsKey(key) ? locks.get(key) : new Object();
 }
 private static void init() {
 if (locks == null) {
 try {
 locks = new HashMap<String, Object>();
 Properties properties = new Properties();
 properties.load(SyncFactory.class.getClassLoader().getResourceAsStream("lock.properties"));
 for (Map.Entry<Object, Object> entry : properties.entrySet()) {
 // key=true => locking with ine object
 // key=false => lock object should always be new
 if (Boolean.parseBoolean(String.valueOf(entry.getValue()))) {
 locks.put((String) entry.getKey(), new Object());
 }
 }
 } catch (IOException e) {
 throw new RuntimeException(e);
 }
 }
 }
}

Service.java actual usage:

public class Service {
 public void foo() {
 synchronized (SyncFactory.getLock("foo")) {
 // do some logic
 }
 }
 public void bar() {
 synchronized (SyncFactory.getLock("bar")) {
 // do some logic
 }
 }
}

• user4619: What do you think you are accomplishing with this?

Answer 1

1. Your large outer if inside init() makes me a sad panda, why don't you just put a conditional return right in the beginning of the method? Still I'm curious whether you can provide a use-case for this approach.
2. Why even lazy? Do you expect no getLock() calls in most cases when your app is ran?

• ykhrustalev: 1. There are two kind of people who do return first and who write if. As for the real world example I can't provide one but who knows what could happen in future and might be it would be mainstream. 2. It is done with a lazy initialization just for that example in real life I would leave it for the IOC container

Answer 2

Do you use synchronization to ensure visibility and prevent parallel running of two (or more) pieces of code? If yes, I agree with Dmitry Makarenko, I also don't see a real use-case of this class. The coder knows which pieces of code use the same shared data when they write the code, so runtime configuration is unnecessary.

If you use it only to prevent parallel running of two (or more) piece of code which do not access the same shared data it could be fine but SemaphoreFactory would be a better, more intuitive name for this class.

Two other notes:

• Using Properties.stringPropertyNames and getProperty instead of entrySet would make the init method easier to read (no casting required) without a real performance loss (since it runs only once).

• Using a guard clause in the init method would make the code flatten:

  if (locks != null) {
   return;
  }
  try {
   ...
  } catch (IOException e) {
   throw new RuntimeException(e);
  }
  

  See: Flattening Arrow Code

Answer 3

I just don't see a point in this factory. It has nothing in common with locking except class name, getLock method and "lock.properties" string inside. It's just a global hash map. You always can (should) use synchronized with this or even better with private ivar. Any need too use your factory makes me think there is a bad design.

• ykhrustalev: the point is to have configurable way to apply lock on the same object and have actually sync or crate new object each time and have no sync

Answer 4

Java 5 Semaphore is ready made, tested, and secure tool for your problem.

You certainly find something on the Net searching "semaphore java lock"

• ykhrustalev: Sorry I don't get how it could help. Your approach uses acquiring lock to control the state of sync, but it still requires some logic to apply the restrictions when to acquire or release the locking state on particular method.
• cl-r: No matter, I was not certain to be right for the way you are looking for, just tried to be helpful if can take advantage on resctriction and constrains pointed by Semaphore. However it seems you can use synchronized docs.oracle.com/javase/1.4.2/docs/api/java/util/Hashtable.html

Answer 5

Everyone else has failed to mention:

Locks on non-final references are less than useless as they do not guarantee anything.

Right now, in both cases, you just have all the overhead of synchronized with none of the guarantees! But worse, naive maintainers will think there is a guarantee when there is none.

Thread Safe and non-Thread Safe versions of classes exist for a reason. Whatever you think you are accomplishing with this, especially if it is performance related, you are misguided.

• ykhrustalev: oh dear, it was sarcasm back in 2012, this code makes no sense, it was for fun only, but still thanks for paying attention to that