6576763: Thread constructors throw undocumented NPE for null name
authorchegar
Fri, 29 Aug 2008 17:46:45 +0100
changeset 1148 1e917f49e503
parent 1147 c77d563839c4
child 1149 1e32392ecafa
6576763: Thread constructors throw undocumented NPE for null name Summary: update javadoc to specify NPE as well as fix minor bug in implementation. Reviewed-by: alanb
jdk/src/share/classes/java/lang/Thread.java
jdk/test/java/lang/ThreadGroup/NullThreadName.java
--- a/jdk/src/share/classes/java/lang/Thread.java	Wed Aug 27 10:28:26 2008 -0700
+++ b/jdk/src/share/classes/java/lang/Thread.java	Fri Aug 29 17:46:45 2008 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright 1994-2007 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 1994-2008 Sun Microsystems, Inc.  All Rights Reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -30,7 +30,6 @@
 import java.security.PrivilegedAction;
 import java.util.Map;
 import java.util.HashMap;
-import java.util.Collections;
 import java.util.concurrent.locks.LockSupport;
 import sun.misc.SoftCache;
 import sun.nio.ch.Interruptible;
@@ -120,6 +119,10 @@
  * Every thread has a name for identification purposes. More than
  * one thread may have the same name. If a name is not specified when
  * a thread is created, a new name is generated for it.
+ * <p>
+ * Unless otherwise noted, passing a {@code null} argument to a constructor
+ * or method in this class will cause a {@link NullPointerException} to be
+ * thrown.
  *
  * @author  unascribed
  * @see     Runnable
@@ -348,6 +351,10 @@
      */
     private void init(ThreadGroup g, Runnable target, String name,
                       long stackSize) {
+        if (name == null) {
+            throw new NullPointerException("name cannot be null");
+        }
+
         Thread parent = currentThread();
         SecurityManager security = System.getSecurityManager();
         if (g == null) {
@@ -379,7 +386,6 @@
             }
         }
 
-
         g.addUnstarted();
 
         this.group = g;
@@ -403,160 +409,175 @@
         tid = nextThreadID();
     }
 
-   /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(null, null,</code>
-     * <i>gname</i><code>)</code>, where <b><i>gname</i></b> is
-     * a newly generated name. Automatically generated names are of the
-     * form <code>"Thread-"+</code><i>n</i>, where <i>n</i> is an integer.
-     *
-     * @see     #Thread(ThreadGroup, Runnable, String)
+    /**
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (null, null, gname)}, where {@code gname} is a newly generated
+     * name. Automatically generated names are of the form
+     * {@code "Thread-"+}<i>n</i>, where <i>n</i> is an integer.
      */
     public Thread() {
         init(null, null, "Thread-" + nextThreadNum(), 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(null, target,</code>
-     * <i>gname</i><code>)</code>, where <i>gname</i> is
-     * a newly generated name. Automatically generated names are of the
-     * form <code>"Thread-"+</code><i>n</i>, where <i>n</i> is an integer.
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (null, target, gname)}, where {@code gname} is a newly generated
+     * name. Automatically generated names are of the form
+     * {@code "Thread-"+}<i>n</i>, where <i>n</i> is an integer.
      *
-     * @param   target   the object whose <code>run</code> method is called.
-     * @see     #Thread(ThreadGroup, Runnable, String)
+     * @param  target
+     *         the object whose {@code run} method is invoked when this thread
+     *         is started. If {@code null}, this classes {@code run} method does
+     *         nothing.
      */
     public Thread(Runnable target) {
         init(null, target, "Thread-" + nextThreadNum(), 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(group, target,</code>
-     * <i>gname</i><code>)</code>, where <i>gname</i> is
-     * a newly generated name. Automatically generated names are of the
-     * form <code>"Thread-"+</code><i>n</i>, where <i>n</i> is an integer.
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (group, target, gname)} ,where {@code gname} is a newly generated
+     * name. Automatically generated names are of the form
+     * {@code "Thread-"+}<i>n</i>, where <i>n</i> is an integer.
      *
-     * @param      group    the thread group.
-     * @param      target   the object whose <code>run</code> method is called.
-     * @exception  SecurityException  if the current thread cannot create a
-     *             thread in the specified thread group.
-     * @see        #Thread(ThreadGroup, Runnable, String)
+     * @param  group
+     *         the thread group. If {@code null} and there is a security
+     *         manager, the group is determined by {@linkplain
+     *         SecurityManager#getThreadGroup SecurityManager.getThreadGroup()}.
+     *         If there is not a security manager or {@code
+     *         SecurityManager.getThreadGroup()} returns {@code null}, the group
+     *         is set to the current thread's thread group.
+     *
+     * @param  target
+     *         the object whose {@code run} method is invoked when this thread
+     *         is started. If {@code null}, this thread's run method is invoked.
+     *
+     * @throws  SecurityException
+     *          if the current thread cannot create a thread in the specified
+     *          thread group
      */
     public Thread(ThreadGroup group, Runnable target) {
         init(group, target, "Thread-" + nextThreadNum(), 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(null, null, name)</code>.
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (null, null, name)}.
      *
-     * @param   name   the name of the new thread.
-     * @see     #Thread(ThreadGroup, Runnable, String)
+     * @param   name
+     *          the name of the new thread
      */
     public Thread(String name) {
         init(null, null, name, 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(group, null, name)</code>
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (group, null, name)}.
      *
-     * @param      group   the thread group.
-     * @param      name    the name of the new thread.
-     * @exception  SecurityException  if the current thread cannot create a
-     *               thread in the specified thread group.
-     * @see        #Thread(ThreadGroup, Runnable, String)
+     * @param  group
+     *         the thread group. If {@code null} and there is a security
+     *         manager, the group is determined by {@linkplain
+     *         SecurityManager#getThreadGroup SecurityManager.getThreadGroup()}.
+     *         If there is not a security manager or {@code
+     *         SecurityManager.getThreadGroup()} returns {@code null}, the group
+     *         is set to the current thread's thread group.
+     *
+     * @param  name
+     *         the name of the new thread
+     *
+     * @throws  SecurityException
+     *          if the current thread cannot create a thread in the specified
+     *          thread group
      */
     public Thread(ThreadGroup group, String name) {
         init(group, null, name, 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object. This constructor has
-     * the same effect as <code>Thread(null, target, name)</code>.
+     * Allocates a new {@code Thread} object. This constructor has the same
+     * effect as {@linkplain #Thread(ThreadGroup,Runnable,String) Thread}
+     * {@code (null, target, name)}.
      *
-     * @param   target   the object whose <code>run</code> method is called.
-     * @param   name     the name of the new thread.
-     * @see     #Thread(ThreadGroup, Runnable, String)
+     * @param  target
+     *         the object whose {@code run} method is invoked when this thread
+     *         is started. If {@code null}, this thread's run method is invoked.
+     *
+     * @param  name
+     *         the name of the new thread
      */
     public Thread(Runnable target, String name) {
         init(null, target, name, 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object so that it has
-     * <code>target</code> as its run object, has the specified
-     * <code>name</code> as its name, and belongs to the thread group
-     * referred to by <code>group</code>.
-     * <p>
-     * If <code>group</code> is <code>null</code> and there is a
-     * security manager, the group is determined by the security manager's
-     * <code>getThreadGroup</code> method. If <code>group</code> is
-     * <code>null</code> and there is not a security manager, or the
-     * security manager's <code>getThreadGroup</code> method returns
-     * <code>null</code>, the group is set to be the same ThreadGroup
-     * as the thread that is creating the new thread.
+     * Allocates a new {@code Thread} object so that it has {@code target}
+     * as its run object, has the specified {@code name} as its name,
+     * and belongs to the thread group referred to by {@code group}.
      *
-     * <p>If there is a security manager, its <code>checkAccess</code>
-     * method is called with the ThreadGroup as its argument.
-     * <p>In addition, its <code>checkPermission</code>
-     * method is called with the
-     * <code>RuntimePermission("enableContextClassLoaderOverride")</code>
+     * <p>If there is a security manager, its
+     * {@link SecurityManager#checkAccess(ThreadGroup) checkAccess}
+     * method is invoked with the ThreadGroup as its argument.
+     *
+     * <p>In addition, its {@code checkPermission} method is invoked with
+     * the {@code RuntimePermission("enableContextClassLoaderOverride")}
      * permission when invoked directly or indirectly by the constructor
-     * of a subclass which overrides the <code>getContextClassLoader</code>
-     * or <code>setContextClassLoader</code> methods.
-     * This may result in a SecurityException.
-
-     * <p>
-     * If the <code>target</code> argument is not <code>null</code>, the
-     * <code>run</code> method of the <code>target</code> is called when
-     * this thread is started. If the target argument is
-     * <code>null</code>, this thread's <code>run</code> method is called
-     * when this thread is started.
-     * <p>
-     * The priority of the newly created thread is set equal to the
+     * of a subclass which overrides the {@code getContextClassLoader}
+     * or {@code setContextClassLoader} methods.
+     *
+     * <p>The priority of the newly created thread is set equal to the
      * priority of the thread creating it, that is, the currently running
-     * thread. The method <code>setPriority</code> may be used to
-     * change the priority to a new value.
-     * <p>
-     * The newly created thread is initially marked as being a daemon
+     * thread. The method {@linkplain #setPriority setPriority} may be
+     * used to change the priority to a new value.
+     *
+     * <p>The newly created thread is initially marked as being a daemon
      * thread if and only if the thread creating it is currently marked
-     * as a daemon thread. The method <code>setDaemon </code> may be used
-     * to change whether or not a thread is a daemon.
+     * as a daemon thread. The method {@linkplain #setDaemon setDaemon}
+     * may be used to change whether or not a thread is a daemon.
      *
-     * @param      group     the thread group.
-     * @param      target   the object whose <code>run</code> method is called.
-     * @param      name     the name of the new thread.
-     * @exception  SecurityException  if the current thread cannot create a
-     *               thread in the specified thread group or cannot
-     *               override the context class loader methods.
-     * @see        Runnable#run()
-     * @see        #run()
-     * @see        #setDaemon(boolean)
-     * @see        #setPriority(int)
-     * @see        ThreadGroup#checkAccess()
-     * @see        SecurityManager#checkAccess
+     * @param  group
+     *         the thread group. If {@code null} and there is a security
+     *         manager, the group is determined by {@linkplain
+     *         SecurityManager#getThreadGroup SecurityManager.getThreadGroup()}.
+     *         If there is not a security manager or {@code
+     *         SecurityManager.getThreadGroup()} returns {@code null}, the group
+     *         is set to the current thread's thread group.
+     *
+     * @param  target
+     *         the object whose {@code run} method is invoked when this thread
+     *         is started. If {@code null}, this thread's run method is invoked.
+     *
+     * @param  name
+     *         the name of the new thread
+     *
+     * @throws  SecurityException
+     *          if the current thread cannot create a thread in the specified
+     *          thread group or cannot override the context class loader methods.
      */
     public Thread(ThreadGroup group, Runnable target, String name) {
         init(group, target, name, 0);
     }
 
     /**
-     * Allocates a new <code>Thread</code> object so that it has
-     * <code>target</code> as its run object, has the specified
-     * <code>name</code> as its name, belongs to the thread group referred to
-     * by <code>group</code>, and has the specified <i>stack size</i>.
+     * Allocates a new {@code Thread} object so that it has {@code target}
+     * as its run object, has the specified {@code name} as its name,
+     * and belongs to the thread group referred to by {@code group}, and has
+     * the specified <i>stack size</i>.
      *
      * <p>This constructor is identical to {@link
      * #Thread(ThreadGroup,Runnable,String)} with the exception of the fact
      * that it allows the thread stack size to be specified.  The stack size
      * is the approximate number of bytes of address space that the virtual
      * machine is to allocate for this thread's stack.  <b>The effect of the
-     * <tt>stackSize</tt> parameter, if any, is highly platform dependent.</b>
+     * {@code stackSize} parameter, if any, is highly platform dependent.</b>
      *
      * <p>On some platforms, specifying a higher value for the
-     * <tt>stackSize</tt> parameter may allow a thread to achieve greater
+     * {@code stackSize} parameter may allow a thread to achieve greater
      * recursion depth before throwing a {@link StackOverflowError}.
      * Similarly, specifying a lower value may allow a greater number of
      * threads to exist concurrently without throwing an {@link
@@ -564,9 +585,9 @@
      * the relationship between the value of the <tt>stackSize</tt> parameter
      * and the maximum recursion depth and concurrency level are
      * platform-dependent.  <b>On some platforms, the value of the
-     * <tt>stackSize</tt> parameter may have no effect whatsoever.</b>
+     * {@code stackSize} parameter may have no effect whatsoever.</b>
      *
-     * <p>The virtual machine is free to treat the <tt>stackSize</tt>
+     * <p>The virtual machine is free to treat the {@code stackSize}
      * parameter as a suggestion.  If the specified value is unreasonably low
      * for the platform, the virtual machine may instead use some
      * platform-specific minimum value; if the specified value is unreasonably
@@ -574,9 +595,9 @@
      * maximum.  Likewise, the virtual machine is free to round the specified
      * value up or down as it sees fit (or to ignore it completely).
      *
-     * <p>Specifying a value of zero for the <tt>stackSize</tt> parameter will
+     * <p>Specifying a value of zero for the {@code stackSize} parameter will
      * cause this constructor to behave exactly like the
-     * <tt>Thread(ThreadGroup, Runnable, String)</tt> constructor.
+     * {@code Thread(ThreadGroup, Runnable, String)} constructor.
      *
      * <p><i>Due to the platform-dependent nature of the behavior of this
      * constructor, extreme care should be exercised in its use.
@@ -588,15 +609,32 @@
      *
      * <p>Implementation note: Java platform implementers are encouraged to
      * document their implementation's behavior with respect to the
-     * <tt>stackSize parameter</tt>.
+     * {@code stackSize} parameter.
+     *
+     *
+     * @param  group
+     *         the thread group. If {@code null} and there is a security
+     *         manager, the group is determined by {@linkplain
+     *         SecurityManager#getThreadGroup SecurityManager.getThreadGroup()}.
+     *         If there is not a security manager or {@code
+     *         SecurityManager.getThreadGroup()} returns {@code null}, the group
+     *         is set to the current thread's thread group.
      *
-     * @param      group    the thread group.
-     * @param      target   the object whose <code>run</code> method is called.
-     * @param      name     the name of the new thread.
-     * @param      stackSize the desired stack size for the new thread, or
-     *             zero to indicate that this parameter is to be ignored.
-     * @exception  SecurityException  if the current thread cannot create a
-     *               thread in the specified thread group.
+     * @param  target
+     *         the object whose {@code run} method is invoked when this thread
+     *         is started. If {@code null}, this thread's run method is invoked.
+     *
+     * @param  name
+     *         the name of the new thread
+     *
+     * @param  stackSize
+     *         the desired stack size for the new thread, or zero to indicate
+     *         that this parameter is to be ignored.
+     *
+     * @throws  SecurityException
+     *          if the current thread cannot create a thread in the specified
+     *          thread group
+     *
      * @since 1.4
      */
     public Thread(ThreadGroup group, Runnable target, String name,
@@ -669,6 +707,7 @@
      * @see     #stop()
      * @see     #Thread(ThreadGroup, Runnable, String)
      */
+    @Override
     public void run() {
         if (target != null) {
             target.run();
@@ -1386,28 +1425,25 @@
      * Returns the context ClassLoader for this Thread. The context
      * ClassLoader is provided by the creator of the thread for use
      * by code running in this thread when loading classes and resources.
-     * If not set, the default is the ClassLoader context of the parent
-     * Thread. The context ClassLoader of the primordial thread is
-     * typically set to the class loader used to load the application.
+     * If not {@linkplain #setContextClassLoader set}, the default is the
+     * ClassLoader context of the parent Thread. The context ClassLoader of the
+     * primordial thread is typically set to the class loader used to load the
+     * application.
      *
-     * <p>First, if there is a security manager, and the caller's class
-     * loader is not null and the caller's class loader is not the same as or
-     * an ancestor of the context class loader for the thread whose
-     * context class loader is being requested, then the security manager's
-     * <code>checkPermission</code>
-     * method is called with a
-     * <code>RuntimePermission("getClassLoader")</code> permission
-     *  to see if it's ok to get the context ClassLoader..
+     * <p>If a security manager is present, and the invoker's class loader is not
+     * {@code null} and is not the same as or an ancestor of the context class
+     * loader, then this method invokes the security manager's {@link
+     * SecurityManager#checkPermission(java.security.Permission) checkPermission}
+     * method with a {@link RuntimePermission RuntimePermission}{@code
+     * ("getClassLoader")} permission to verify that retrieval of the context
+     * class loader is permitted.
      *
-     * @return the context ClassLoader for this Thread
+     * @return  the context ClassLoader for this Thread, or {@code null}
+     *          indicating the system class loader (or, failing that, the
+     *          bootstrap class loader)
      *
-     * @throws SecurityException
-     *        if a security manager exists and its
-     *        <code>checkPermission</code> method doesn't allow
-     *        getting the context ClassLoader.
-     * @see #setContextClassLoader
-     * @see SecurityManager#checkPermission
-     * @see RuntimePermission
+     * @throws  SecurityException
+     *          if the current thread cannot get the context ClassLoader
      *
      * @since 1.2
      */
@@ -1428,21 +1464,22 @@
     /**
      * Sets the context ClassLoader for this Thread. The context
      * ClassLoader can be set when a thread is created, and allows
-     * the creator of the thread to provide the appropriate class loader
-     * to code running in the thread when loading classes and resources.
-     *
-     * <p>First, if there is a security manager, its <code>checkPermission</code>
-     * method is called with a
-     * <code>RuntimePermission("setContextClassLoader")</code> permission
-     *  to see if it's ok to set the context ClassLoader..
+     * the creator of the thread to provide the appropriate class loader,
+     * through {@code getContextClassLoader}, to code running in the thread
+     * when loading classes and resources.
      *
-     * @param cl the context ClassLoader for this Thread
+     * <p>If a security manager is present, its {@link
+     * SecurityManager#checkPermission(java.security.Permission) checkPermission}
+     * method is invoked with a {@link RuntimePermission RuntimePermission}{@code
+     * ("setContextClassLoader")} permission to see if setting the context
+     * ClassLoader is permitted.
      *
-     * @exception  SecurityException  if the current thread cannot set the
-     * context ClassLoader.
-     * @see #getContextClassLoader
-     * @see SecurityManager#checkPermission
-     * @see RuntimePermission
+     * @param  cl
+     *         the context ClassLoader for this Thread, or null  indicating the
+     *         system class loader (or, failing that, the bootstrap class loader)
+     *
+     * @throws  SecurityException
+     *          if the current thread cannot set the context ClassLoader
      *
      * @since 1.2
      */
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/ThreadGroup/NullThreadName.java	Fri Aug 29 17:46:45 2008 +0100
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6576763
+ * @summary (thread) Thread constructors throw undocumented NPE for null name
+ */
+
+/*
+ * Verify that threads constructed with a null thread name do not get added
+ * to the list of unstarted thread for a thread group. We can do this by
+ * checking that a daemon threadGroup is desroyed after its final valid thread
+ * has completed.
+ */
+
+import java.util.concurrent.CountDownLatch;
+import static java.lang.System.out;
+
+public class NullThreadName
+{
+    static CountDownLatch done = new CountDownLatch(1);
+
+    public static void main(String args[]) throws Exception {
+        ThreadGroup tg = new ThreadGroup("chegar-threads");
+        Thread goodThread = new Thread(tg, new GoodThread(), "goodThread");
+        try {
+            Thread badThread = new Thread(tg, new Runnable(){
+                @Override
+                public void run() {} }, null);
+        } catch (NullPointerException npe) {
+            out.println("OK, caught expected " + npe);
+        }
+        tg.setDaemon(true);
+        goodThread.start();
+
+        done.await();
+
+        int count = 0;
+        while (goodThread.isAlive()) {
+            /* Hold off a little to allow the thread to complete */
+            out.println("GoodThread still alive, sleeping...");
+            try { Thread.sleep(2000); }
+            catch (InterruptedException unused) {}
+
+            /* do not wait forever */
+            if (count++ > 5)
+                throw new AssertionError("GoodThread is still alive!");
+        }
+
+        if (!tg.isDestroyed()) {
+            throw new AssertionError("Failed: Thread group is not destroyed.");
+        }
+    }
+
+    static class GoodThread implements Runnable
+    {
+        @Override
+        public void run() {
+            out.println("Good Thread started...");
+            out.println("Good Thread finishing");
+            done.countDown();
+        }
+    }
+}