0

I've stumbled upon this interesting tutorial on multiple client-server chat model. I understood everything except for one thing. In the implementation of class MultiThreadChatServerSync, he explains that we need to synchronize parts of the code because of the different threads that are sharing the same resource which is the field

private final clientThread[] threads;

I've read plenty of articles on how synchronized(this) works. How is this code synchronizing the blocks of statement if 'this' refers to an instance of a thread? Let's say thread[0] enters a synchronized(this) statement so it obtains the monitor of itself. Meanwhile thread[1] enters a synchronized(this) statement aswell but it wouldn't get blocked because it would be obtaining the monitor of its own instance and not thread[0]'s monitor. Can someone explain to me why this code is synchronized? Am I missing something?

Here is a link to the article. I am unable to link to the specific part. So just search for the sentence 'public class MultiThreadChatServerSync'. http://makemobiapps.blogspot.com/p/multiple-client-server-chat-programming.html

Here is the code for those who cannot access the webpage.

import java.io.DataInputStream;
import java.io.PrintStream;
import java.io.IOException;
import java.net.Socket;
import java.net.ServerSocket;

/*
 * A chat server that delivers public and private messages.
 */
public class MultiThreadChatServerSync {

  // The server socket.
  private static ServerSocket serverSocket = null;
  // The client socket.
  private static Socket clientSocket = null;

  // This chat server can accept up to maxClientsCount clients' connections.
  private static final int maxClientsCount = 10;
  private static final clientThread[] threads = new       clientThread[maxClientsCount];

  public static void main(String args[]) {
    // The default port number.
    int portNumber = 2222;
    if (args.length < 1) {
      System.out.println("Usage: java MultiThreadChatServerSync <portNumber>\n" + "Now using port number=" + portNumber);
    } else {
      portNumber = Integer.valueOf(args[0]).intValue();
    }

    /*
     * Open a server socket on the portNumber (default 2222). Note that we can
     * not choose a port less than 1023 if we are not privileged users (root).
     */
    try {
      serverSocket = new ServerSocket(portNumber);
    } catch (IOException e) {
      System.out.println(e);
    }

    /*
     * Create a client socket for each connection and pass it to a new client
     * thread.
     */
    while (true) {
      try {
        clientSocket = serverSocket.accept();
        int i = 0;
        for (i = 0; i < maxClientsCount; i++) {
          if (threads[i] == null) {
            (threads[i] = new clientThread(clientSocket, threads)).start();
            break;
          }
        }
        if (i == maxClientsCount) {
          PrintStream os = new PrintStream(clientSocket.getOutputStream());
          os.println("Server too busy. Try later.");
          os.close();
          clientSocket.close();
        }
      } catch (IOException e) {
        System.out.println(e);
      }
    }
  }
}
/*
 * The chat client thread. This client thread opens the input and the output
 * streams for a particular client, ask the client's name, informs all the
 * clients connected to the server about the fact that a new client has joined
 * the chat room, and as long as it receive data, echos that data back to all
 * other clients. The thread broadcast the incoming messages to all clients and
 * routes the private message to the particular client. When a client leaves the
 * chat room this thread informs also all the clients about that and terminates.
 */
class clientThread extends Thread {

  private String clientName = null;
  private DataInputStream is = null;
  private PrintStream os = null;
  private Socket clientSocket = null;
  private final clientThread[] threads;
  private int maxClientsCount;

  public clientThread(Socket clientSocket, clientThread[] threads) {
    this.clientSocket = clientSocket;
    this.threads = threads;
    maxClientsCount = threads.length;
  }

  public void run() {
    int maxClientsCount = this.maxClientsCount;
    clientThread[] threads = this.threads;

    try {
      /*
       * Create input and output streams for this client.
       */
      is = new DataInputStream(clientSocket.getInputStream());
      os = new PrintStream(clientSocket.getOutputStream());
      String name;
      while (true) {
        os.println("Enter your name.");
        name = is.readLine().trim();
        if (name.indexOf('@') == -1) {
          break;
        } else {
          os.println("The name should not contain '@' character.");
        }
      }

      /* Welcome the new the client. */
      os.println("Welcome " + name
      + " to our chat room.\nTo leave enter /quit in a new line.");
      synchronized (this) {
        for (int i = 0; i < maxClientsCount; i++) {
          if (threads[i] != null && threads[i] == this) {
            clientName = "@" + name;
            break;
          }
        }
        for (int i = 0; i < maxClientsCount; i++) {
          if (threads[i] != null && threads[i] != this) {
            threads[i].os.println("*** A new user " + name + " entered the chat room !!! ***");
          }
        }
      } 
      /* Start the conversation. */
      while (true) {
        String line = is.readLine();
        if (line.startsWith("/quit")) {
          break;
        }
        /* If the message is private sent it to the given client. */
        if (line.startsWith("@")) {
          String[] words = line.split("\\s", 2);
          if (words.length > 1 && words[1] != null) {
            words[1] = words[1].trim();
            if (!words[1].isEmpty()) {
              synchronized (this) {
                for (int i = 0; i < maxClientsCount; i++) {
                  if (threads[i] != null && threads[i] != this
                      && threads[i].clientName != null
                      && threads[i].clientName.equals(words[0])) {
                    threads[i].os.println("<" + name + "> " + words[1]);
                    /*
                     * Echo this message to let the client know the private
                     * message was sent.
                     */
                    this.os.println(">" + name + "> " + words[1]);
                    break;
                  }
                }
              }
            }
          }
        } else {
          /* The message is public, broadcast it to all other clients. */
          synchronized (this) {
            for (int i = 0; i < maxClientsCount; i++) {
              if (threads[i] != null && threads[i].clientName != null) {
                threads[i].os.println("<" + name + "> " + line);
              }
            }
          }
        }
      }
      synchronized (this) {
        for (int i = 0; i < maxClientsCount; i++) {
          if (threads[i] != null && threads[i] != this && threads[i].clientName != null) {
            threads[i].os.println("*** The user " + name + " is leaving the chat room !!! ***");
          }
        }
      }
      os.println("*** Bye " + name + " ***");

      /*
       * Clean up. Set the current thread variable to null so that a new client
       * could be accepted by the server.
       */
      synchronized (this) {
        for (int i = 0; i < maxClientsCount; i++) {
          if (threads[i] == this) {
            threads[i] = null;
          }
        }
      }
      /*
       * Close the output stream, close the input stream, close the socket.
       */
      is.close();
      os.close();
      clientSocket.close();
    } catch (IOException e) {
    }
  }
}
Patrick
  • 191
  • 2
  • 8
  • They are different this's. What if you called them `a` and `b` instead of using this. – matt Jan 06 '17 at 13:43
  • Exactly my point. They are different this's. That means the code is actually not synchronized. Am I right? – Patrick Jan 06 '17 at 13:44
  • the code is really convoluted there: it creates an array of Threads like itself within the run method, clientThread[] threads = this.threads;. It doesn't look nice. – Raul Guiu Jan 06 '17 at 13:49
  • They are synchronized, but not on each other. The blocks could be entered simultaneously. – matt Jan 06 '17 at 13:51
  • The author explains "All synchronized(this){} statements exclude mutually each other. It means, that when a thread enters the synchronized(this){} statement it verifies first that any other synchronized(this){} statement is not being executed by another thread. If a such a statement is being executed by a thread, then this thread, as well as all other threads trying to execute a synchronized(this){} statement, are forced to wait until the thread executing the synchronized(this){} terminates this statement." Is this really the case? – Patrick Jan 06 '17 at 14:16
  • 1
    I don't think the author of that piece has understood synchronized correctly. Access to the `threads` array is not being protected, two separate threads could modify the array at the same time. The code is not safe. It could be made safe by synchronizing on a single object -the array, a `static` instance of `Object`, or even the class itself (`synchronized(clientThread.class)`) – Stik Jan 06 '17 at 14:55
  • That's what I was thinking as well. It got me really confused especially since he gave a non-synchronized version and a synchronized version. Nevertheless, still learned a lot from the article. – Patrick Jan 06 '17 at 15:15

1 Answers1

0

the author states that "All synchronized(this){} statements exclude mutually each other. It means,..."

If you take it out of context, that statement is blatantly false.

The only context that could make it true is if the author is talking about all of the synchronized(this) {...} statements in some particular example, and all of them are in instance methods belonging to the same class, and only one instance of the class is ever in use at any given time.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
  • So do you think it would've better to use synchronized(ClassNameOfObject){}? – Patrick Jan 07 '17 at 03:28
  • @Patrick, What's important is that each _invariant_ that your program needs to protect is always guarded by the same lock object. An invariant is some relationshp between data in your program that must always be true. For example, the number of items in a queue must always equal the number of items added minus the number that have been removed. If you have two different instances of the same queue class, that's two distinct invariants. It's OK to use one lock to protect more than one invariant, but it never makes sense to use more than one lock for a single invariant. – Solomon Slow Jan 07 '17 at 22:08
  • The code in the example is ugly, and I don't want to spend a lot of time studying it. From the couple of minutes I've spent looking at it, it's not at all clear to me what the invariants are that the author is trying to protect. – Solomon Slow Jan 07 '17 at 22:10
  • Thank you for that explanation, it clears things up. But just to be sure let's say I have an array of objects of the same class that extends the Threads class. That class has a method with a synchronized(this){} statement. If all those objects are running at the same time and then they call that particular method, they are obtaining different locks.. am I right? Thus it wouldn't be synchronized? – Patrick Jan 08 '17 at 02:19
  • @Patrick; Yes, that's right. The keyword, `this` is only allowed in instance methods, and it always refers to an instance of the class. If you have different instances of the same class, then different threads will be allowed to synchronize on those different instances at the same time. Synchronizing on `this` is good when all of the variables that you want to protect are_ instance variables_ of the class, but it's not sufficient if any `static` variables are involved: You'll want to synchronize on some global object (e.g., on the Class itself) in that case. – Solomon Slow Jan 09 '17 at 15:29