StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

POJava Thread Safety Issue in Struts ScopeInterceptor class?
text
Body
copied!<p>I'm trying to understand if there is a thread-safety issue inside of Struts2 ScopeInterceptor class (/org/apache/struts2/interceptor/ScopeInterceptor.java), here's the code in question:</p> <pre><code> private static Map locks = new IdentityHashMap(); static final void lock(Object o, ActionInvocation invocation) throws Exception { synchronized (o) { int count = 3; Object previous = null; while ((previous = locks.get(o)) != null) { if (previous == invocation) { return; } if (count-- <= 0) { locks.remove(o); o.notify(); throw new StrutsException("Deadlock in session lock"); } o.wait(10000); } ; locks.put(o, invocation); } } static final void unlock(Object o) { synchronized (o) { locks.remove(o); o.notify(); } } </code></pre> <p>I have a Websphere application showing 45 stalled threads, high cpu usage. 33 threads are stalled at "locks.remove(o)" inside of "unlock" method. The other 12 threads are stalled inside of "locks.get(o)" inside of "lock" method.</p> <p>It seems to me that the usage of IdentityHashMap is thread-unsafe. Could simply wrapping IdentityHashMap with Collections.synchronizedMap() solve this problem?:</p> <pre><code> private static Map locks = Collections.synchronizedMap(new IdentityHashMap()); static final void lock(Object o, ActionInvocation invocation) throws Exception { synchronized (o) { int count = 3; Object previous = null; while ((previous = locks.get(o)) != null) { if (previous == invocation) { return; } if (count-- <= 0) { locks.remove(o); o.notify(); throw new StrutsException("Deadlock in session lock"); } o.wait(10000); } ; locks.put(o, invocation); } } static final void unlock(Object o) { synchronized (o) { locks.remove(o); o.notify(); } } </code></pre> <p>It seems to me that the author tried to "fix" IdentityHashMap's synchronization problem by using synchronized code blocks, however that doesn't protect against multiple threads if the Object "o" is a thread-specific object. And, since the code blocks within lock and unlock are separate, then IdentityHashMap will (and does!) get called simultaneously by more than one thread (as per our Java core evidence).</p> <p>Is the Collections.synchronizedMap() wrapper the correct fix, or am I missing something?</p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload