Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6251838

(spec thread) Misleading example usage code in java.lang.ThreadLocal

XMLWordPrintable

    • generic, x86
    • generic, linux

      A DESCRIPTION OF THE PROBLEM :
      The class description contains the following example usage code:


       public class SerialNum {
           // The next serial number to be assigned
           private static int nextSerialNum = 0;

           private static ThreadLocal serialNum = new ThreadLocal() {
               protected synchronized Object initialValue() {
                   return new Integer(nextSerialNum++);
               }
           };

           public static int get() {
               return ((Integer) (serialNum.get())).intValue();
           }
       }
       

      What's confusing is the role of the *synchronized* keyword in
      the initialValue() override. You should clearly state what it is
      that you're protecting when using that keyword--especially in
      a class like this one. In this case, you're protecting the serial
      number from concurrent access.

      Again, why is this confusing? Because this might lead readers
      to believe that their ThreadLocal.initialValue() overrides must
      always synchronize on the ThreadLocal object itelf. Not only
      is that untrue, it is bad practice.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      This version synchronize's on SerialNum itself, rather
      than on the thread local variable. (In a real application
      it would be even better if SerialNum synchronized
      on a private static lock object so that no user could
      deadlock the SerialNum class, but here, the goal is
      clarity...)

       public class SerialNum {
           // The next serial number to be assigned
           private static int nextSerialNum = 0;

           private static ThreadLocal serialNum = new ThreadLocal() {
               protected Object initialValue() {
                   int next;
                   synchronized (SerialNum.class) {
                       next = nextSerialNum++;
                   }
                   return new Integer(next);
               }
           };

           public static int get() {
               return ((Integer) (serialNum.get())).intValue();
           }

       }
       
      ACTUAL -
      See the description.

      URL OF FAULTY DOCUMENTATION :
      http://java.sun.com/j2se/1.5.0/docs/api/java/lang/ThreadLocal.html
      ###@###.### 2005-04-07 08:13:53 GMT
      Although there is a plan to systematically generify all Thread* code examples that were overlooked during Tiger, here is a specific example for this program. It still needs error correction but shows more modern usage of ThreadLocal. Merging this with the suggestions above will result in a "suggested fix."

      class SerialNum {
          private static ThreadLocal<Integer> serialNum
      = new ThreadLocal<Integer>()
          {
      // The next serial number to be assigned
      private int nextSerialNum = 0;

      protected synchronized Integer initialValue() {
                  return nextSerialNum++;
              }
          };

          public static int get() {
              return serialNum.get();
          }
      }

            psoper Pete Soper (Inactive)
            ndcosta Nelson Dcosta (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

              Created:
              Updated:
              Resolved:
              Imported:
              Indexed: