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