0

I have a multithreaded server project to implement in Java.
How the message is stored:
Every message that is produced by the clients and comes through the sockets to the server is stored in a String variable called "commandString" which is a member variable of class "Command". Every time a message arrives at the server, a new "Command" object is created and the message is stored in "commandString".
How the message is appended to the file:
So, the server is keeping an ArrayList of the Commands which is passed to a thread whose only purpose is to append every new Command object of the ArrayList to a Database.txt file. It is really important that every Command object is written to the txt file one by one as soon as the object is created.
What is happening instead:
When I try to run the project, the file writing feature does not work at all, meaning that nothing is ever written in the file, while the thread is created and being run just fine. But when I launch the debugger with a breakpoint as shown below, every object is being written as soon as the next message arrives. This means I am going to miss the last message which won't be written to the file.
How can I make it write to the file every object as soon as it is created?

public class DataBase implements Runnable {
  ArrayList<Command> cmdQueue;
  public int lastSize;  

  @Override
  public void run() {
    try {   
      this.setLastSize(this.getCmdQueue().size());
      FileWriter fw = new FileWriter(BlackBox.createBlackBox(),true);   
      BufferedWriter bfw = new BufferedWriter(fw);
      this.setLastSize(0);
        
      while(true) {
        if(lastSize <this.cmdQueue.size() && this.cmdQueue.size() !=0) {
          bfw.write(this.cmdQueue.get(this.cmdQueue.size()-1).getCommandString());
          bfw.flush();
          this.setLastSize(this.cmdQueue.size());
          bfw.close();
        } 
      }
    } catch(IOException ioe) {
      System.out.println(ioe.getMessage());
      ioe.printStackTrace();
    }   
  }//end of run()

In App.java:

    //the "server" object receives the messages using threads that are already created
    DataBase = new DataBase(server.cmdQueue);
    dBase.setCmdQueue(server.cmdQueue);
    Thread bBoxThread = new Thread(bBase);
    bBoxThread.start();

Feel free to speak you minds.Thank for your time.

Jim Rhodes
  • 5,021
  • 4
  • 25
  • 38
petrunko
  • 43
  • 7
  • Shouldnt you close the BufferedWriter? Also, the while loop should be ``while (true)`` – NomadMaker Jun 09 '21 at 13:56
  • @NomadMaker, yes you are right. In the original code I use a boolean variable from an external config file, so when it set to true, the file writting is activated. Also I close the BufferedWriter but is was so low on the document that I forgot to include it here. Anything else that looks sketchy to you? – petrunko Jun 09 '21 at 14:05
  • You're always writing the last string. What happens if two commands are recieved before you get a chance to write either one? – NomadMaker Jun 09 '21 at 14:07
  • @JimRhodes, I am going to agree with #6 as also NomadMaker pointed out. The code has been simplified as much as it could since no one is going to go through the whole project which is actually a couple of thousand lines long. I also find very hopeful the fact that you people dedicated the time to read the whole text of my post. Also, all the constructors and setters that you see as duplications have been added there after hours of debugging to avoid NPE's. – petrunko Jun 09 '21 at 14:19
  • Your biggest fundamental problem seems to be that you're using `ArrayList` as a queue. It's neither a `Queue`, nor is it thread-safe. Now [ArrayBlockingQueue](https://stackoverflow.com/q/26543807/2541560) on the other hand... – Kayaman Jun 09 '21 at 14:25
  • You mention threads more than once, but you don't show how items are added to the queue. Do you properly synchronize your threads? Lack of synchronization can possibly cause the issues you see. This is especially compelling considering that it works in the debugger - which has to synchronize everything in order for you to see the values of your variables. – Chris Parker Jun 09 '21 at 14:25
  • @NomadMaker, thats a fair question, this flaw could actually cause me to miss a message . Another idea i already tried, was to overwrite the whole list to the file every time a new object is added, but for some reason that didn't work either. – petrunko Jun 09 '21 at 14:26
  • @ChrisParker, I agree with you, in a project like that I am expected to deal with that kind of sync problems all the time, but how is it possible for the file to contain nothing after the termination of the App, considering the fact that I gave it enough time for the ammount of messages received? Thats why I believe the problem is in this piece of code. – petrunko Jun 09 '21 at 14:30
  • @Kayaman, thats a possible solution, but then it means the above piece of code is fine? Why doesn't it write absolutely nothing to the file when I run the project ? – petrunko Jun 09 '21 at 14:35
  • No, the above code is absolutely broken. It's not thread-safe. It's possible for the database thread to never see any messages in the `ArrayList` (or just see the size as `0` which is effectively the same thing), if you don't ensure visibility with synchronization or by a thread-safe data structure like `BlockingQueue`. – Kayaman Jun 09 '21 at 14:35
  • @Kayaman, ok , then why when I launch it on the debugger, one object is written to the file only if and when a client sends the next message ? Am I doing anything with the flushing wrong or what ? – petrunko Jun 09 '21 at 14:47
  • Debugging affects concurrency, so it can/will run differently under a debugger and without it, making concurrency bugs hard to debug. You have no idea what *real* bugs you have left before you fix the concurrency issues. – Kayaman Jun 09 '21 at 14:55
  • @JimRhodes the problem is not that I am closing the bufferedWriter on every loop because I am creating a new one on every loop. I am doing this becuase close() also acts as flush() for the buffer. Also I wasn't given any suggestions about if this spesifficaly shown piece of code is correct or not when it comes to file writting. Do not prompt me to post the actual two thousand lines of code if your are not willing to review them. – petrunko Jun 09 '21 at 14:59
  • @kayaman , yes I agree , but the visibillity of the ArrayList to the dataBase thread is ensured and moreover you cannot correct it if I dont show you the complete code. I think I got my answer, the bug is not in these 20 lines of code. Thank you all. – petrunko Jun 09 '21 at 15:03
  • If anyone actually wants to review the whole project, send me an email containing your Github username at stackeroverload@gmail.com. But again, I dont think you will. – petrunko Jun 09 '21 at 15:10
  • The problem with a lack of synchronization is that one thread may *never* see changes made in another thread. If one process is planting messages in a queue, and a different thread is removing them, then it's entirely possible that the remove side cannot ever see your change. As I said in my first comment, the debugger forces some amount of synchronization so that it can show you values. Use proper thread-safe tools and proper synchronization and almost certainly your problem will go away. – Chris Parker Jun 09 '21 at 16:06

0 Answers0