Strange thread behavior

Question

I'm trying to do this: The question is displayed in the console. If during some time the user does not write the answer, then the next question is asked. If the user enters an answer, the next question is asked immediately. My code:

public class Test {
private boolean stopQuestion;
Thread          scannerThread = new Thread();

public static void main(final String[] args) {
    final Test test = new Test();

    test.scannerThread = new Thread(new Runnable() {
        @Override
        public void run() {
            try {
                String string;
                do {
                    string = test.requestInput(new Thread(new Runnable() {

                        @Override
                        public void run() {
                            try {
                                Thread.sleep(3000);
                            } catch (final InterruptedException e) {
                            }
                            test.scannerThread.interrupt();

                        }
                    }));
                } while (!test.stopQuestion);

                System.out.println("Input: " + string);
            } catch (final IOException e) {
                throw new RuntimeException(e);
            }
        }
    });
    test.scannerThread.start();

}

public String requestInput(final Thread timer) throws IOException {
    final BufferedReader br = new BufferedReader(new InputStreamReader(System.in));

    timer.start();

    System.out.println("Any question");
    System.out.println("Please type your answer: ");
    try {
        while (!br.ready()) {
            Thread.sleep(100);
        }
    } catch (final InterruptedException e) {
        System.out.println("Time is over. Next question: ");
        return null;
    }
    System.out.println("Thank You for providing input!");
    return br.readLine();
}

}

If you do not write anything to the console, everything seems to work as expected. Time ends and the next question is asked. But if something is written to the console, the timer starts to malfunction and the next question does not wait for the specified amount of time, sometimes it does not wait at all. I do not understand what's the matter.

I created instance of thread outside the method and pass instance to the method as reference but then throws IllegalThreadStateException.

There appear to be several things wrong here, but the two major things that stick out are the visibility of `stopQuestion` and the whole idea of accessing `System.in` from multiple threads. Oh, and `sleep()` can't be used for accurate timing either, there are absolutely no guarantees tied to it. — biziclop, May 30 '17 at 15:47
Also, if user has typed half of an answer, and you then print another question, the typed text is still there in the input buffer, so that's going to be *very* confusing. Your premise is flawed, i.e. what you're trying to do is not going to work with a console program. — Andreas, May 30 '17 at 15:53
And I've just noticed that you use `interrupt()` for presumably repeated signalling, which doesn't sound like a good idea either. — biziclop, May 30 '17 at 15:53

score 0 · Answer 1 · answered May 30 '17 at 15:53

0

I see two major problems with your code:

You are continously creating threads that are supposed to read input:

            do {
                string = test.requestInput(new Thread(new Runnable() {

                    @Override
                    public void run() {
                        try {
                            Thread.sleep(3000);
                        } catch (final InterruptedException e) {
                            e.printStackTrace();
                        }
                        test.scannerThread.interrupt();

                    }
                }));
            } while (!test.stopQuestion); // <-- this is always true

You are opening as many BufferedReaders on System.in as many timer threads you are launching:
```
final BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
```

Also, you are not closing any of these BufferedReader instances.

answered May 30 '17 at 15:53

syntagma

23,346
16
78
134

If you close the `BufferedReader`, you close `System.in`, and prevent all future reading of input. Do not close readers / scanners / inputstreams wrapped over `System.in`. – Andreas May 30 '17 at 15:54
There should be only one instance of `BufferedReader` and it should be closed eventually. – syntagma May 30 '17 at 15:56
1

It should never be closed. `System.in` is managed by the Java runtime, and should not be closed by the app. – Andreas May 30 '17 at 15:57
Any source on that? I've looked up few sources and it seems a good idea to close the `BufferedReader` before the app's termination. – syntagma May 30 '17 at 15:59
If you read from `System.in` directly, i.e. not wrapped, would you also add a `System.in.close()` at the end of the program? Very likely not, since you didn't open it. So why do you think it's a "good idea" to close `System.in` just because you wrapped it in helpers that made it easier to read text? What's *your* source of "good idea"? – Andreas May 30 '17 at 16:02
The important part here is the wrapper over `System.in`. My source is mainly practice, suppose we would have an interface on the left side of the assignmnent and we would have our own imlpementation of buffered reader. That implementation may assume that `close()` will be called and in its implementation of `close()` it may free some additional resources. – syntagma May 30 '17 at 16:08
I'm not saying you absolutely can't do it. I said you *shouldn't*, since you're violating normal resource management. Unless otherwise stated, whoever opens a resource is responsible for closing it. Invoked methods using the resource should never close it, *unless* the resource responsibility is *explicitly* handed off. It's all part of the [Separation of Concern](https://en.wikipedia.org/wiki/Separation_of_concerns) design principle. Anyway, that's my source: SoC. – Andreas May 30 '17 at 16:08
If your wrapper needs closing, to free its resources, you need to prevent that close call from reaching `System.in`, e.g. as described in question "[Closing BufferedReader and System.in](https://stackoverflow.com/q/8203981/5221149)", where answer is to use a `CloseShieldInputStream`. – Andreas May 30 '17 at 16:11
You are not violating resource management, because you are closing the *wrapper*, not `System.in` (and if the implementation of `BufferedReader` will close `System.in` then it is OK). – syntagma May 30 '17 at 16:11
Unless you use something like `CloseShieldInputStream`, closing the wrapper *will* close `System.in`, and *that* is violating SoC. If your method adding the wrapper was part of a bigger program, you'd close `System.in` and break the rest of the program. It's why there are so very many questions on here with that problem, when people close `Scanner` used in one method, and try reading more in another method. Sure, in a small program you can violate SoC, which is why I said "*shouldn't*", but it's good to start adhering to Soc principles, so they become second nature. – Andreas May 30 '17 at 16:14
I closed BufferedReader but it did not solve the problem. Timer does not work correctly after input. Can many threads be created and they work simultaneously? – Comfmore May 30 '17 at 16:18
Closing `BufferedReader` was less important. The points is that you are endlessly creating new threads and new `BufferedReaders` when you should do it once for each question. – syntagma May 30 '17 at 16:19
I tried to create one BufferedReader in the main method and passed it as a parameter to the input, but nothing has changed. – Comfmore May 30 '17 at 16:22
That's because you still have problem (1). – syntagma May 30 '17 at 16:23
In the code above, I create one anonymous thread for each question, but I can not close them. Also I can not create one thread and restart it. – Comfmore May 30 '17 at 16:27

Strange thread behavior

1 Answers1