How to tell if in a synchronised block in Java?

This must have a simple answer but I just don't know what it is... If I, say, do the following in Java:

class First{
    public void first(){
        Second second=new Second();

        synchronized(this){
            second.second(this);
        }

        second.second(this);
    }
}

How do I check in Second.second that the synchronised lock has been obtained before this method is called, and perhaps throw an exception if this is not the case? For instance:

class Second{
    public void second(First first){
        if(!/*want to test that lock obtained for first, but don't know how*/){
            throw new RuntimeException("Must lock first!");
        }
    }
}

I want the second call to Second.second to throw the RuntimeException if this is not obvious to the code above.

There's a method for that

public void second(First first) {
    if (!Thread.holdsLock(first)) throw new IllegalStateException("Lock required");
}

HOWEVER, you don't want this.

What you want is this:

public void second(First first) {
    synchronized (first) {
        // do stuff
    }
}

If a thread holds a lock and you then sync on that again, that's free, and doesn't break anything: It takes no time, and does not freeze up your thread. you can re-fetch a lock your thread already holds. Java maintains a counter. (locks are reentrant).

It seems silly to demand that callers acquire this lock; why not just acquire it yourself? If caller already acquired it, no problem. No time lost, code continues to function.

NB: As far as code style goes, throwing RuntimeException is bad, and putting an exclamation point in the message is very bad (think about it; 90%+ of all messages in exceptions would otherwise end in a ! and it's going to get real annoying to review logs). I'd argue that you don't need braces for early exits like this either. So if you must go with the 'check and throw' style, the snippet is written with some fixes applied for you

> I want the second call to Second.second to throw the RuntimeException
> if this is not obvious to the code above.

I think this is a bad idea. Either you want First to be handling the lock and second not caring, or Second to be handling the lock irrespective of whether it is First or Third.

If we compare it to classes in the standard library, we might look at HashMap vs ConcurrentMap. HashMap is a non-threadsafe class - that is, it is identical to Second in your example. ConcurrentMap is a "threadsafe" class - that is, it handles its own synchronized operations.

This really depends on what constitutes "threadsafe", so requires more knowledge of how the class is being used to know whether the ConcurrentMap method of thread-safety will actually provide thread-safety.

Does anyone other than First have access to the same instance of Second and you are guarding against multi-threaded access from this angle? If so, the ConcurrentMap approach might be more appropriate. Do multiple operations occur on Second itself in a multi-threaded environment? If so, manually locking in First would be more appropriate.

Example of multiple operations on Second using map.

Map<Integer, String> map...
... // lets say map has 3 elements by this point and there are 2 threads running.
if (map.size() < 4)
{ // <--- thread may switch here, so both threads are inside the if block
    map.put(map.size(), "This map is too small");
  // Both threads have put in "This map is too small" to the map.
}

For this simple snippet, whether the map is HashMap or ConcurrentMap, we cannot prevent "This map is too small" being added twice. So despite ConcurrentMap providing "thread-safety", this code is not actually thread-safe. Hence, we need an external lock:

...
synchronized (map)
{
   if (map.size() < 4)
   {
      map.add(map.size(), "This map is too small");
   }
}

So given this scenario, ConcurrentMap will provide no benefit, and using the simpler HashMap would be the correct choice.