views:

676

answers:

6

The stop(), suspend(), and resume() in java.lang.Thread are deprecated because they are unsafe. The Sun recommended work around is to use Thread.interrupt(), but that approach doesn't work in all cases. For example, if you are call a library method that doesn't explicitly or implicitly check the interrupted flag, you have no choice but to wait for the call to finish.

So, I'm wondering if it is possible to characterize situations where it is (provably) safe to call stop() on a Thread. For example, would it be safe to stop() a thread that did nothing but call find(...) or match(...) on a java.util.regex.Matcher?

(If there are any Sun engineers reading this ... a definitive answer would be really appreciated.)

EDIT: Answers that simply restate the mantra that you should not call stop() because it is deprecated, unsafe, whatever are missing the point of this question. I know that that it is genuinely unsafe in the majority of cases, and that if there is a viable alternative you should always use that instead.

This question is about the subset cases where it is safe. Specifically, what is that subset?

A: 

Maybe there's something I don't know, but as java.sun.com said, it is unsafe because anything this thread is handling is in serious risk to be damaged. Other objects, connections, opened files... for obvious reasons, like "don't shut down your Word without saving first".

For this find(...) exemple, I don't really think it would be a catastrophe to simply kick it away with a sutiless .stop()...

Havenard
Yea ... that is my gut feeling as well. But gut feeling is not good enough when you are trying to build robust servers.
Stephen C
It's madness to be calling a deprecated method if anything that isn't for fun.
Noon Silk
@silky: it is madness to rule something out *just because* someone else says it is madness :-)
Stephen C
@Stephen C haha, touche, okay I take that :) But in this case it's the Java Designers telling you; and they're more authorative on their language then either of us :)
Noon Silk
@silky: But they don't say it is **always** unsafe, do they?
Stephen C
They say why its unsafe but not clearly enough to get where it is not. -- Same happens in C with functions like strcat() and sprintf(), you must comprehend Buffer Overflows to realize why it is considered unsafe, deprecated, and how to use it safely.
Havenard
@Havenard: and the point of my question is to try to get a clear(er) picture of when it >>is<< safe!
Stephen C
A: 

All forms of concurrency control can be provided by the Java synchronization primitives by constructing more complex concurrency controls that suit your problem.

The reasons for deprecation are clearly given in the link you provide. If you're willing to accept the reasons why, then feel free to use those features.

However, if you choose to use those features, you also accept that support for those features could stop at any time.

Edit: I'll reiterate the reason for deprecation as well as how to avoid them.

Since the only danger is that objects that can be referenced by the stoped thread could be corrupted, simply clone the String before you pass it to the Thread. If no shared objects exist, the threat of corrupted objects in the program outside the stoped Thread is no longer there.

Ben S
@Ben: but what if the complex concurrency solutions simply do not solve the problem? For example: http://stackoverflow.com/questions/1282824/how-to-set-a-time-limit-on-a-java-function-running-a-regex
Stephen C
@Stephen C: I think in such cases, the `Thread` which requires arbitrary interruption should be implemented as a separate `Process` instead, and forcibly shut down using termination signals to be done safely. I've added my answer to the question you referred to.
Ben S
Even cloning the String doesn't absolutely guarantee safety. You don't know whether the String class does anything clever with the string representations. Does it share buffers for identical strings? Does it cache internal pointers to substrings to optimise searches? When you call myString.find() what processing does it do?
djna
@djna: "You don't know whether the String class does anything clever with the string representations." Yes, I do. It's open source. Also, by convention, the clone method performs a deep copy of objects which eliminates any possible shared state. This can be seen in the String implementation.
Ben S
@djna: If the String class was not open source, you could pass a copy of a `char[]` containing the string instead.
Ben S
@Ben S. What do we "know"? If we have the source we can by inspection determine whether any code used in a thread can safely be cancelled. For each new release of that source we must perform that inspection. But my point here was that in general we don't have the source for libraries, and even apparently trivial interfaces may have deep **unsafe** complexity.
djna
@Ben S. How you create the String is not important to my argument. the String implementation could on construction do all manner of optimisations, such as spotting that it already has seen that array of chars. It could when doing a find() within a String build a little dictionary. We can't infer from the interface what is going on. Simple example of an implementation surprise **x = new Integer(8); y = new Integer(8);** x and y actually point to the same Integer!! There's a cunning internal optimisation for small Integers.
djna
@djna: The `Integer` optimization is well documented. Because caching like this can cause those issues all of the standard Java classes have clear documentation on whether or not objects are cached. I trust that the Sun engineers have documented their caching, if in doubt, read up on all the classes you use.
Ben S
@Ben S - Maybe it was a bad example. The point I was trying to make is that a reading of the interface cannot in itself indicate that the cancellation is safe. I thought you were asserting that cloning a String was guranteed to result in a safe cancellation situation. I was trying to illustrate that there can be linkages "under the covers" that can't be directly inferred from the interface. Short of trustworthy documentation we can only look at the implementation.
djna
@djna: clearly we don't have trustworthy documentation on this point, but I believe that there are other ways to approach this problem.
Stephen C
+2  A: 

The lack of safety comes from the idea idea of critical sections

Take mutex

do soem work, temporarily while we work our state is inconsistent

// all consistent now

Release mutex

If you blow away the thread and it happend to be in a critical section then the object is left in an inconsistent state, that means not safely usable from that point.

For it to be safe to kill the thread you need to understand the entire processing of whatever is being done in that thread, to know that there are no such critical sections in the code. If you are using library code, then you may not be able to see the source and know that it's safe. Even if it's safe today it may not be tomorrow.

(Very contrived) Example of possible unsafety. We have a linked list, it's not cyclic. All the algorithms are really zippy because we know it's not cyclic. During our critical section we temporarily introduce a cycle. We then get blown away before we emerge from the critical section. Now all the algorithms using the list loop forever. No library author would do that surely! How do you know? You cannot assume that code you use is well written.

In the example you point to, it's surely possible to write the requreid functionality in an interruptable way. More work, but possible to be safe.

I'll take a flyer: there is no documented subset of Objects and methods that can be used in cancellable threads, because no library author wants to make the guarantees.

djna
@djna: Re your last point. I wouldn't rely on documentation anyway. Rather, I'd want to use a static code analysis tool to ensure that a thread can be safely stopped; e.g. see my answer.
Stephen C
+3  A: 

Here's my attempt at answering my own question.

I think that the following conditions should be sufficient for a single thread to be safely stoped using Thread.stop():

  1. The thread execution must not create or mutate any state (i.e. Java objects, class variables, external resources) that might be visible to other threads in the event that the thread is stopped.
  2. The thread execution must not use notify to any other thread during its normal execution.
  3. The thread must not start or join other threads, or interact with then using stop, suspend or resume.

(The term thread execution above covers all application-level code and all library code that is executed by the thread.)

The first condition means that a stopped thread will not leave any external data structures or resources in an inconsistent state. This includes data structures that it might be accessing (reading) within a mutex. The second condition means that a stoppable thread cannot leave some other thread waiting. But it also forbids use of any synchronization mechanism other that simple object mutexes.

A stoppable thread must have a way to deliver the results of each computation to the controlling thread. These results are created / mutated by the stoppable thread, so we simply need to ensure that they are not visible following a thread stop. For example, the results could be assigned to private members of the Thread object and "guarded" with a flag that is atomically by the thread to say it is "done".

EDIT: These conditions are pretty restrictive. For example, for a "regex evaluator" thread to be safely stopped, if we must guarantee that the regex engine does not mutate any externally visible state. The problem is that it might do, depending on how you implement the thread!

  1. The Pattern.compile(...) methods might update a static cache of compiled patterns, and if they did they would (should) use a mutex to do it. (Actually, the OpenJDK 6.0 version doesn't cache Patterns, but Sun might conceivably change this.)
  2. If you try to avoid 1) by compiling the regex in the control thread and supplying a pre-instantiated Matcher, then the regex thread does mutate externally visible state.

In the first case, we would probably be in trouble. For example, suppose that a HashMap was used to implement the cache and that the thread was interrupted while the HashMap was being reorganized.

In the second case, we would be OK provided that the Matcher had not been passed to some other thread, and provided that the controller thread didn't try to use the Matcher after stopping the regex matcher thread.

So where does this leave us?

Well, I think I have identified conditions under which threads are theoretically safe to stop. I also think that it is theoretically possible to statically analyse the code of a thread (and the methods it calls) to see if these conditions will always hold. But, I'm not sure if this is really practical.

Does this make sense? Have I missed something?

Stephen C
The use of reflection APIs would allow the executing code to be selected dynamically, hence potentially defeating the static analysis, so perhaps you would need to prohibit that?
djna
If you blow away a thread that happens to be in a synchronized method, and consequently perhaps blocking some other thread, do we know that Java on the death of the first thread will ensure that the other thread will run?
djna
The problem for me with static analysis adding information above the documented API is that your application design depends upon "uncontracted" behaviour - the library author has not promised that their next release will not do something (or use another library that does something) unsafe.
djna
A: 

If my understanding is right, the problem has to do with synchronization locks not being released as the generated ThreadInterruptedException() propagates up the stack.

Taking that for granted, it's inherently unsafe because you can never know whether or not any "inner method call" you happened to be in at the very moment stop() was invoked and effectuated, was effectively holding some synchronization lock, and then what the java engineers say is, seemingly, unequivocally right.

What I personally don't understand is why it should be impossible to release any synchronization lock as this particular type of Exception propagates up the stack, thereby passing all the '}' method/synchronization block delimiters, which do cause any locks to be release for any other type of exception.

I have a server written in java, and if the administrator of that service wants a "cold shutdown", then it is simply NECESSARY to be able to stop all running activity no matter what. Consistency of any object's state is not a concern because all I'm trying to do is to EXIT. As fast as I can.

Erwin Smout
-1 this is wrong. The problem is that locks are released, but that there is nothing to guarantee that the data-structures they protected are in a consistent state.
Stephen C
A: 

A concrete example would probably help here. If anyone can suggest a good alternative to the following use of stop I'd be very interested. Re-writing java.util.regex to support interruption doesn't count.

import java.util.regex.*;
import java.util.*;

public class RegexInterruptTest {

    private static class BadRegexException extends RuntimeException { }
        final Thread mainThread = Thread.currentThread();
        TimerTask interruptTask = new TimerTask() {
            public void run() {
                System.out.println("Stopping thread.");
                // Doesn't work:
                // mainThread.interrupt();
                // Does work but is deprecated and nasty
                mainThread.stop(new BadRegexException());
            }
        };

        Timer interruptTimer = new Timer(true);
        interruptTimer.schedule(interruptTask, 2000L);

        String s = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab";
        String exp = "(a+a+){1,100}";
        Pattern p = Pattern.compile(exp);
        Matcher m = p.matcher(s);
        try {
            System.out.println("Match: " + m.matches());
            interruptTimer.cancel();
        } catch(BadRegexException bre) {
            System.out.println("Oooops");
        } finally {
            System.out.println("All over");
        }
    }
}
Richard Wheeldon
The only alternative would be to run the regex match in a separate JVM that can be killed. (Which is pretty expensive ... ). However, this looks like the kind of use-case where `Thread.stop()` might be safe.
Stephen C