On 8/2/2012 11:52 AM, Yu Lin wrote:
My name is Yu Lin. I'm a Ph.D. student in the CS department at
UIUC. I'm currently doing research on mining Java concurrent library
misuses. I found some uses of concurrent collections in OpenJDK7u may
result in potential atomicity violation bugs or harm the performance.
....

Another possible atomicity vioaltion may happen at line 892 in
jdk/src/share/classes/sun/font/FileFontStrike.java:

L891  if (boundsMap == null) {
L892      boundsMap = new ConcurrentHashMap<Integer, Rectangle2D.Float>();
L893  }
       // some<key, value>  pairs are put into "boundsMap"

A possible buggy scenario is both threads T1 and T2 finds "boundsMap"
is null at the same time (line 891). After T1 creates a new empty map
and puts some<key, value>  pairs into "boundsMap", T2 also creates an
empty map and writes to "boundsMap". Then the<key, value>  pairs put
by T1 will be lost. If this scenario may happen and lead to bug, we
have to add synchronization when initializing "boundsMap".

I haven't looked at the rest but I'll comment on that one.
This is deliberate. The tradeoff is all uses being synchronized versus
the small chance that two threads try to create this map at the same
time, which really doesn't matter. Its just a tiny bit of lost work.
We could have a debate about how uncontended synchronization
is cheap but adding  it somewhat defeats the purpose of using
ConcurrentHashMap. Also if there are two threads active here,
meriting the synchronization then the lock probably is contended ..

Also I am not sure how your patch below would work.
Synchronizing on (null) isn't likely to fix anything.
Instead you just introduced a NullPointerException.

Finally jdk8-dev is probably the "main" list these days.

-phil.

I append a patch to show how to fix the above four problems. I can also list
all of such code I found if you need.

Regards,
Yu

===============================================================

diff -r 7daf4a4dee76 src/share/classes/sun/font/FileFontStrike.java
--- a/src/share/classes/sun/font/FileFontStrike.java    Mon May 07
14:59:29 2012 -0700
+++ b/src/share/classes/sun/font/FileFontStrike.java    Thu Aug 02
11:50:46 2012 -0500
@@ -887,9 +887,10 @@
       * up in Java code either.
       */
      Rectangle2D.Float getGlyphOutlineBounds(int glyphCode) {
-
-        if (boundsMap == null) {
-            boundsMap = new ConcurrentHashMap<Integer, Rectangle2D.Float>();
+        synchronized (boundsMap) {
+            if (boundsMap == null) {
+                boundsMap = new ConcurrentHashMap<Integer,
Rectangle2D.Float>();
+            }
          }

          Integer key = Integer.valueOf(glyphCode);

Reply via email to