deadlock on synchronized ( String intern())

Question

I user sun jdk 1.5 ThreadPoolExecutor( 24, 24,60,TimeUnit.SECONDS, new LinkedBlockingQueue()). soemtime I use jdb tool to find the status of all threads in thread pool are " waiting in a monitor", the code is :

    String key = getKey(dt.getPrefix(), id);
    synchronized (key.intern()) {      ----->

Is there a problem in "synchronized (key.intern()) " ?

I get following informatnio using jdb tool, the status of 24 threads is "waiting in a monitor", it means 24 threads are deadlock at "key.intern()".

(java.lang.Thread)0x28 pool-3-thread-2 waiting in a monitor

(java.lang.Thread)0x27 pool-3-thread-3 waiting in a monitor

(java.lang.Thread)0x1b pool-3-thread-4 waiting in a monitor

(java.lang.Thread)0x1a pool-3-thread-5 waiting in a monitor

(java.lang.Thread)0x19 pool-3-thread-6 waiting in a monitor

(java.lang.Thread)0x18 pool-3-thread-7 waiting in a monitor

(java.lang.Thread)0x17 pool-3-thread-8 waiting in a monitor ...

so the result is : in multi-threads environment, Sting intern() method may be deadlock, ok ?

First quesion is what do you want to achive? What is the problem? Why do you need to synchronize? — Ivan Dubrov, Dec 08 '08 at 11:27
have you looked at threads OUTSIDE the threadpool to see if they are also waiting in a monitor? You should pay attention to the exact object that each thread is synchronized on too. — James Schek, Dec 08 '08 at 17:08
There are two threads outide the threadpool are waiting on a monitor, and also monitor the same line (key.intern() ). — user44230, Dec 12 '08 at 03:11
Synchronizing in interned strings doesn't work at all. It does not guarantee the same string object is returned in different threads, as there is no "happens before" relationship with the intern operation. Thus the two threads have different tables of interned strings until they pass through a monitor on the same object (or otherwise establish "happens before") I've actually seen code where two threads with String.intern() obtain objects with different System.identityHashCode() but identical string values... it happened about 1/3 of the time (in that case). — Gus, Mar 01 '16 at 19:07

score 5 · Answer 1 · edited May 23 '17 at 11:46

5

I posted a related question to this once that you might want to take a look at: Problem with synchronizing on String objects?

What I learned was: using intern'ed Strings for synchronization is a bad practice.

edited May 23 '17 at 11:46

Community

1
1

answered Dec 08 '08 at 14:54

matt b

138,234
66
282
345

3

It would be help full if you detail why it is a bad practice. – Guillaume Coté Jul 12 '11 at 18:20
doesn't the linked question answer that? *when running JMeter tests against the webapp to simulate the expected load, I saw the used heap size grow to almost 1GB in just under 20 minutes.* – matt b Jul 12 '11 at 19:34
5

It would be a better answer if you quickly explain why in the answer instead of letting people read a long question. – Guillaume Coté Jul 15 '11 at 18:40

score 4 · Answer 2 · answered Dec 08 '08 at 09:28

4

Quite. The problem is that key.intern() isn’t really that unique because it’s returning a string from a pool. String.intern() might return the same object even when used on different objects. Try using key itself or a different object altogether.

answered Dec 08 '08 at 09:28

Bombe

81,643
20
123
127

score 2 · Answer 3 · answered Dec 08 '08 at 12:52

2

The code is almost certainly trying to synchronize actions that affect the same key. So it's calling intern() to ensure that the same key gets mapped to the same object, and therefore is valid as an object for synchronization.

The problem, if you're running into a bottleneck there (it's not a deadlock) is that you have too many operations coming in at the same time using the same key.

Rethink what needs to be synchronized.

answered Dec 08 '08 at 12:52

kdgregory

38,754
10
77
102

How did you figure out he was talking about bottleneck? – Miserable Variable Dec 08 '08 at 13:25
How do you know it's not a deadlock. Using something universally accessible like an interned string is an easy way to get deadlock. – erickson Dec 11 '08 at 05:22
the definition of a deadlock is that two (or more) threads are waiting for resources owned by the other(s). This implies two synchronized blocks (or a loop). The code shown, while it could be in a loop, seems to be a pretty straightforward critical-section gate. – kdgregory Dec 13 '08 at 16:15

score 2 · Answer 4 · answered Dec 08 '08 at 13:07

2

If you need to synchronize on a String, don't use a String instance as the mutex (interned or not). A string can be used to create a good mutex object, though: synchronizing on an ID.

answered Dec 08 '08 at 13:07

McDowell

107,573
31
204
267

score 2 · Answer 5 · answered Dec 08 '08 at 13:18

2

You are having two problems. The one is using String as the lock. The second one is deadlock.

If you using String as lock, you will lose the control of "who" and "where" will take that object lock.

Your deadlock issue, which may or may not caused by lock on String. However, the actual reason of deadlock is: "Your code can lead deadlock.". If it can happen, it will happen.

You must trace your threads' stacks to resolve deadlocks.

answered Dec 08 '08 at 13:18

Dennis C

24,511
12
71
99

What is wrong with using String.intern() as lock? I have myself used it successfully – Miserable Variable Dec 08 '08 at 13:27
1

Of course it’s possible to use a String as a lock. However, for exactly the reasons mentioned in the answer it is seldom “good” (whatever that means) to do so. – Bombe Dec 08 '08 at 13:47
Not sure I understand what is bad about it. That is causes deadlock? Or that it is a bottleneck? I don't think either is true. – Miserable Variable Dec 08 '08 at 13:58
1

The point is that completely unrelated code can occupy “your” lock because it happens to use String.intern(), too—which results in the same object being used as the lock. – Bombe Dec 08 '08 at 17:01

score 2 · Answer 6 · answered Dec 08 '08 at 15:03

There is not enough code here to tell what is going wrong. It could be a bottleneck as has been mentioned, but at least one thread should be running (with fairly heavy CPU usage) for that to happen, or a thread that has the lock is put to sleep without releasing the lock.

A deadlock is another possibility but that would require synchronization on two separate locks on multiple threads, and you have shown only one lock object here.

It is really impossible to determine without more information.

score 1 · Answer 7 · answered Dec 08 '08 at 11:26

As Bombe says, key.intern() won't necessarily give you a very unique key to synchronize on.

You should be cautious about changing the code, however. You need to understand the locking strategy in the code before changing it. Removing the intern() call may give you code that appears to work correctly but contains a data race that will bite you later.

erickson · Answer 8 · 2008-12-11T05:53:11.737

You very likely have a deadlock.

If you want to avoid deadlocks, every thread must always acquire locks in the same order. When you use String.intern() to get your locks, you are locking on an instance that any code in the entire JVM has access to, and lock on. Most likely, other threads in your own code are deadlocking, but it doesn't have to be.

I'm not sure what you mean in your answer by "key.intern() guarantee uniqueness". The intern() method reduces uniqueness by returning the same object for every string that's equivalent.

  String s1 = new String(new char[] { 'c', 'o', 'm', 'm', 'o', 'n' }).intern();
  String s2 = new String("commo" + (s1.charAt(s1.length() - 1)).intern();
  String s3 = "common";
  if ((s1 == s2) && (s1 == s3))
    System.out.println("There's only one object here.");

The code above will demonstrate that even though you created two unique instances, by interning them, you replaced them with a single, canonical instance.

There's danger any time you use an object that's visible outside your own code as a lock. Try to stick to private members, objects that you don't allow to escape from your own stack, etc.

score 1 · Answer 9 · answered Aug 23 '17 at 04:09

How about using a unique string prefix with the locking value and using the String.intern() in the synchronized block. For example if you want to lock on the string "lock1", use a UUID prefix like this: "85e565b3-d440-46e7-93b6-69ee7e9a63ee-lock1". This type of string should not be already in the intern pool. i.e. chance of deadlock by other code is very low.

score -1 · Answer 10 · answered Dec 08 '08 at 09:43

key.intern() guarantee uniqueness because key.intern() returns a string from String constants pool.

http://java.sun.com/j2se/1.4.2/docs/api/java/lang/String.html#intern() intern

public String intern() Returns a canonical representation for the string object. A pool of strings, initially empty, is maintained privately by the class String.

score -1 · Answer 11 · answered Dec 08 '08 at 11:09

-1

String.intern() is a native method - that might be a cause of the problem.

answered Dec 08 '08 at 11:09

Michael Borgwardt

342,105
78
482
720

Why would it cause the problem? I think the fact that the method is native or not has no impact, it occur before the synchronization starts. – Guillaume Coté Jul 12 '11 at 18:24

deadlock on synchronized ( String intern())

11 Answers11

Linked