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"

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?)

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.

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




Reply via email to