Threads should not be Cloneable

David Holmes David.Holmes at oracle.com
Tue Aug 17 10:42:31 UTC 2010


Fine I give up - no final.

David

Benedict Elliott Smith said the following on 08/17/10 20:31:
> I think he is arguing that probably many people who do not subscribe to 
> this mailing list will have overridden Thread.clone() with an 
> implementation that does not call super.clone(), simply because the 
> "standard" name for methods that copy an object was "clone". And that 
> you will be breaking all of this perfectly valid code simply because you 
> don't like it. Crusades against ugly code (that break existing 
> applications and libraries) don't do the language much service when 
> perhaps even a majority of applications in various industries are 
> written "badly". Even the smartest make bad decisions when learning, and 
> there are plenty of people who aren't the smartest to carry on when 
> those have figured out not to. Plenty of such projects are then 
> "maintained" (i.e. left alone as much as possible) by people who have 
> never or rarely seen the code base, and inflicting this kind of problem 
> on them is unkind at best.
> 
> I think people on this and similar mailing lists might sometimes forget 
> that the majority of users /don't subscribe to it/, and that these 
> people need to be catered to just as much as those that do. And that the 
> people who /don't /subscribe /just may /be the people who are more 
> likely to have made these kinds of errors, and not understand why their 
> previously functional code now doesn't work with a new release of Java.
> 
> 
> 
> On 17 August 2010 11:07, David Holmes <David.Holmes at oracle.com 
> <mailto:David.Holmes at oracle.com>> wrote:
> 
>     Bruce Chapman said the following on 08/17/10 19:17:
> 
>         In the early days of my climb up the java learning curve, well
>         before
>         much body of commonly understood best practice evolved, there
>         are many
>         things which in hindsight are regrettable. Here's two
> 
>         1./ Our highly threaded app had many classes that extended Thread
>         instead of the (now) wiser "implements Runnable"
> 
> 
>     Well Doug Lea and I were touting this since 1996! If you want to
>     define some work define a Runnable, if you want to define a "worker"
>     then extend Thread. :)
> 
> 
>         2./ I thought clone() and Cloneable was the right way to get a 'copy
>         constructor' (it was in the JDK, it must be best practice - right?)
> 
> 
>     Yes that one is best practice right along-side destructors, oops I
>     mean finalizers :)
> 
> 
>         Therefore there is a reasonable chance that in that (once
>         familiar yet
>         minuscule) body of code from a previous millennium, there is a class
>         that extends Thread and is cloned using a clone()  method.
> 
>         You might not find it in more recent code, but these were once
>         commonish
>         idioms, till Josh corrected us with Effective Java. Some of that
>         code
>         may still be running. Don't break it.
> 
> 
>     Well I contend it's already broken if it is cloning a thread but
>     that aside ...
> 
>     ... You seem to be advocating for not making this change at all, as
>     opposed to arguing for or against clone() being final? But we either
>     have to disallow cloning or give it meaningful semantics - and the
>     latter isn't going to happen.
> 
>     David
> 
> 
> 
>         Bruce
> 
> 
>         David Holmes wrote:
> 
>             Jeroen Frijters said the following on 08/17/10 14:11:
> 
>                 David Holmes wrote:
> 
>                     Fortunately, as Brian stated, compatibility is not
>                     an end unto itself
>                     here and we often do have documented
>                     incompatibilities across major
>                     releases. In this case there is far more harm, in my
>                     opinion, leaving
>                     the possibility of people trying to clone threads
>                     than there is in
>                     breaking a hypothetical program that is unlikely to
>                     be functioning
>                     correctly anyway. Thread should never have been
>                     cloneable in any way -
>                     the fact that this has flown under the radar for so
>                     long is a strong
>                     indicator that nobody actually does this in practice
>                     (else they would
>                     have complained that it didn't work).
> 
> 
>                 I really don't understand your position. It clearly
>                 doesn't make sense to call Object.clone() on a Thread,
>                 but you can have a perfectly safe clone() on a Thread
>                 subclass:
> 
>                 public final MyCloneableThread extends Thread {
>                  public Object clone() {
>                    return new MyCloneableThread();
>                  }
>                 }
> 
> 
>             I assume you really meant something like "new
>             MyCloneableThread(this)" to actually get a copy. You can do
>             that, but:
> 
>             a) the above gives you nothing that the constructor alone
>             could not achieve
>             b) the above is only valid in a final class (as used), or if
>             documented explicitly so that subclasses know that they can
>             not use super.clone()
> 
>             If we prevent a Thread subclass from calling super.clone()
>             but still allow the subclass to override clone() then we
>             will need to document that they can only use a
>             construction-based clone technique, and that all further
>             subclasses will also be constrained to that technique.
> 
>             I don't see the point in going to such lengths when our
>             message is a very simple "Thread is not cloneable - get over
>             it, move on". Let's close the door completely, not leave it
>             ajar. I/we only want to set right what should not have been
>             done wrong in the first place.
> 
>                 On the other hand, there is no reason to make clone() in
>                 Thread final other than some vague notion that you want
>                 to prevent people from writing new code like the above,
>                 but given that Java is an "old" and stable platform that
>                 argument doesn't carry much weight either.
> 
>                 BTW, from a security standpoint, overriding clone
>                 doesn't help. An attacker can simply create a Thread
>                 subclass that doesn't have the ACC_SUPER flag set and
>                 that class will be able to call Object.clone() just fine.
> 
> 
>             I'm not quite sure exactly what you mean, but if that is the
>             case then someone should file a bug report.
> 
>             Cheers,
>             David
> 
>                 Regards,
>                 Jeroen
> 
> 
> 
> 
> 



More information about the core-libs-dev mailing list