3

I am writing a small Java class to act a heartbeat for a project. The class is fairly simple. I have a public class called HeartbeatServer.class that contains to private classes which both implement Runnable.

The heartbeat and listener thread will both use the same DatagramSocket which I created outside of the scope of both thread classes and declared it volatile so both threads can access it.

My question is regarding my thread classes. In the thread classes if I call HeartbeatServer.this.serverSocket what is the scope of this? My concern is I don't want each thread to use a different version of HeartbeatServer since I have to bind the DatagramSocket to a specific IP address and port.

Is what I am doing correct to get the result I am looking for? I want both threads to have access to the same DatagramSocket that was created by the constructor of the HeartbeatServer class.

Here is my code.

public volatile DatagramSocket serverSocket;
private Map<String, InetSocketAddress> registeredClients = new HashMap<String, InetSocketAddress>();
public volatile Boolean running = true;

public HeartbeatServer() throws SocketException{

    serverSocket = new DatagramSocket(null);

}

** Other methods would go here **

// This is the thread that will send the heartbeats back to the client.
private class HeartbeatThread implements Runnable {

    @Override
    public void run() {

       while (HeartbeatServer.this.running) {

           HeartbeatServer.this.sendData();

       }

    }

}

// This is the thread that will listen for clients connecting to the server.
private class ListenerThread implements Runnable {

    @Override
    public void run() {

        while (HeartbeatServer.this.running) {

            HeartbeatServer.this.recieveData();

        }

    }

}

** NOTE **

My code is not done, so things might not make any sense in the current context.

Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
AtomicPorkchop
  • 2,625
  • 5
  • 36
  • 55

2 Answers2

0

If you need HeartbeatServer to be unique you should make the HeartbeatServer Singleton. Please check here for more details.

Simply

public class HeartbeatServer {
    private static HeartbeatServer heartbeatServer;

    // Disable object creation from other classes
    private HeartbeatServer () {
    }

    public static HeartbeatServer getInstance() {
          if(heartbeatServer == null)
              heartbeatServer = new HeartbeatServer();
          return heartbeatServer;        
    }
}

What you are doing is, your thread object is using the enclosing parent. Which is not recommended.

Viswanath Lekshmanan
  • 9,945
  • 1
  • 40
  • 64
  • If I am accessing all of my methods through the singleton I created do I need to declare my socket and running variables as volatile? – AtomicPorkchop Oct 22 '15 at 09:20
  • If you are frequently writing the values from multiple threads to the variable in singleton then do that. If its a read only no need for that. – Viswanath Lekshmanan Oct 22 '15 at 09:26
0

If you want to make sure the HeartBeatServer instance you get is the right one, you could add a constructor to the internal runnables, along the lines of

private class HeartbeatThread implements Runnable {

  private HeartbeatServer server;

  HeartBeatThread(HeartbeatServer theServer) {
    this.server = theServer;
  }

  @Override
  public void run() {
     while (server.running) {
       server.sendData();
     }
  }

}
francesco foresti
  • 2,004
  • 20
  • 24