Java: Sharing and calling variables between threads

Question

I made a basic server using sockets and wanted to add a simple GUI to turn it on and off. To make the GUI still work while the server is running a while-loop, i created a thread for the sockets. Now what i was thinking of was adding a boolean to that while-loop which exits said loop and causes the server to stop when pressing a button in the GUI.

Now the problem is that the boolean is set in the GUI thread and needs to be called in the server thread. I read about setting the boolean to volatile or using an AtomicBoolean but neither of them seemed to work. Is there maybe something special i need to look for when calling the boolean from the server thread?

This is the (simplified) code i wrote so far:

public class GUI {

    private static int port = 12345;    
    private static volatile boolean online = false;     

    public static void main(String[] args) {

        //GUI works so i left the code out
        //Basically generates a button giving it the actionlistener below

    }

    private static ActionListener getActionListener() {

        return new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {

                if(!online) {
                    online = true;

                    Thread serverThread = new Thread(new ServerThread());
                    serverThread.start();

                } else {
                    online = false;                 
                }               
            }
        };
    }

    public static boolean getOnlineState() {
        return online;
    }

    public static int getPort() {
        return port;
    }
}

and the class containing the server thread:

public class ServerThread implements Runnable {

    @Override
    public void run() {

        try {
            ServerSocket serSoc = new ServerSocket(GUI.getPort());
            Socket cliSoc = serSoc.accept();
            PrintWriter  out = new PrintWriter(cliSoc.getOutputStream(), true);
            BufferedReader in = new BufferedReader(new    InputStreamReader(cliSoc.getInputStream()));

            String input;
            while(GUI.getOnlineState()) {
                while((input = in.readLine()) != null) {
                    out.println(input);
                }
            }

            out.println("Stopping");
            cliSoc.shutdownInput();
            cliSoc.shutdownOutput();
            cliSoc.close();         
            serSoc.close();
            out.close();
            in.close();

        } catch(IOException e) {
            e.printStackTrace();
        }       
    }   
}

Since i'm new to all this multithreading stuff, there might be some other mistakes which i'd be glad if you would tell me about.

You've several significant issues in this code, not to mention gross over-use of static, but it looks to be your nested while loops are killing you. The outer loop is irrelevant once the inner loop gets started. Don't you want one loop, the inner loop, that checks both the BufferedReader's state and the online state? — Hovercraft Full Of Eels
– Hovercraft Full Of Eels, Commented Aug 22, 2018 at 4:10
readline() also needs to be interrupted. See answers to stackoverflow.com/questions/3595926/… — Alexei Kaigorodov
– Alexei Kaigorodov, Commented Aug 22, 2018 at 4:55

Hovercraft Full Of Eels · Accepted Answer · 2018-08-22 04:31:53Z

1

The nested loop is a problem:

while(GUI.getOnlineState()) {
    while((input = in.readLine()) != null) {
        out.println(input);
    }
}

Once the inner loop is entered, it will continue looping until the input stream is no longer working, and you will have no way of breaking out of it. Perhaps better would be to get rid of the outer loop entirely and combine your logic:

while(GUI.getOnlineState() && (input = in.readLine()) != null) {
    out.println(input);
}

Other unrelated issues are your over-reliance on static fields and methods, something that might work for small toy programs but that risks increasing problems as your program gets bigger and potentially more buggy. Statics risk increasing the connectedness of things that shouldn't be connected, increasing the cyclomatic complexity of your code, and thus the risk of bugs. Isolate your concerns into private instance fields and minimal public methods needed.

answered Aug 22, 2018 at 4:31

Hovercraft Full Of Eels

286k25 gold badges268 silver badges391 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

YNSKLG Over a year ago

okay this is odd... I was wondering about that nested loop too but it didn't work with the combined logic previously so i made it those two loops (didn't really think about it just desperately tried to find a solution) but now it is working so thanks

Collectives™ on Stack Overflow

Java: Sharing and calling variables between threads

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related