0

I found a quite strange thing when I try to send a picture from client side to server side through socket using Java. Here is my code in client side of Class ImgSender which extends Thread class:

public class ImgSender extends Thread
{
    Socket img_s;
    BoolFlag imageready,dataready;
    ImgBoundingBox image;
    ByteArrayOutputStream byteArrayOutputStream;;

public ImgSender(Socket s,BoolFlag dataready,BoolFlag imageready,ImgBoundingBox image)
{
    this.img_s=s;
    this.imageready=imageready;
    this.image=image;
    this.dataready=dataready;
}
public void run()
{
    boolean running=true;
    while(running)
    {
        // System.out.println("Img sender running");
        if(imageready.getFlag())
        { System.out.println("trying to send img");
            try
            {
                  OutputStream outputStream = img_s.getOutputStream();
                  ImageIO.write(image.getImg(), "jpg", outputStream);

                  System.out.println("Send image"+System.currentTimeMillis());
                  outputStream.flush();
                  outputStream.close();
                  imageready.setFlag(false);

            }
            catch(Exception e)
            {
                System.out.println("image client send failed");
                imageready.setFlag(false);
            }
        }
    }    
}

What is strange is that as I comment out the first statement:

System.out.println("Img sender running");

The image won't be sent to the server and "trying to send img" won't be printed out. If I don't comment it out, or , I do some other stuff like sleep() before the if statement, the picture could be sent to the server and "trying to send img" is printed. Moreover, if I debug the programme by putting a break point at:

if(imageready.getFlag())

Then execute it step by step, it would pass the if statement and send it image successfully.

In terms of the server side, here is the code for the Class ImgManager:

private class ImgManager extends Thread
{
    Socket img_s; 
    boolean working=true;

   public ImgManager(Socket s)
   {        
       this.img_s=s;
   }


   public void run()
   {
       while(working)
       {
           try
           {
                   InputStream inputStream = img_s.getInputStream();
                   System.out.println("Reading: " + System.currentTimeMillis());

                   BufferedImage image = ImageIO.read(ImageIO.createImageInputStream(inputStream));

                   System.out.println("Received " + image.getHeight() + "x" + image.getWidth() + ": " + System.currentTimeMillis());
                   ImageIO.write(image, "jpg", new File("img.jpg"));
            }
            catch(Exception e )
            {  
                working=false;
                System.out.println("stopped working socket");
            }

       }
   }

    public void end()
    {
        try
        {
            working=false;
            img_s.close();
        }
        catch(Exception e)
        {
            System.out.println("connection close failed");
        }
    }

}

}
}

When it receive the image, it would print out the size of the picture so that we know if it can get the image.

I tried these two programmes on different machines(mac and pc windows7) and different software(blueJ,eclipse or even launch it with command line). The problem shows no matter in which environment.

Here is the link to my code of the whole project storing in google drive: Server:https://drive.google.com/file/d/0B6w_5_wGgS14UnZCbXJKUGdvSFE/view?usp=sharing Client:https://drive.google.com/file/d/0B6w_5_wGgS14aUlmcG4yQWVnemM/view?usp=sharing

I think it's better to download the source code and look inside it, it's hard to describe everything in here. Sincerely looking for your help!

Kent Lee
  • 107
  • 4
  • 13
  • 1
    You can't close the socket inside a loop and expect the loop to keep working. You don't need to use ImageIO just to copy an image from the socket to a file. Just copy the bytes. I don't see where `"Img sender running"` is printed anywhere. – user207421 May 12 '15 at 18:32
  • 1
    Nobody is going to download the source code. If you can't describe and exhibit your problem clearly here the question has no permanent value and may be deleted. – user207421 May 12 '15 at 18:46
  • 1
    just had a look at boolflag and it is not thread-safe https://code.google.com/p/silkin/source/browse/trunk/code/BoolFlag.java?r=48 use AtomicBoolean – maraca May 12 '15 at 19:13
  • @maraca So great! You are right! It works fine after synchronized the setter and getter! Thank you so much! – Kent Lee May 15 '15 at 13:23
  • @CheuckKwanLee you are welcome. I will write an answer to show why I immediately suspected the BoolFlag. – maraca May 15 '15 at 18:54

1 Answers1

0

The behaviour you described: It works sometimes and sometimes not, depending on the delay or the machine where it is executed usually points to a race condition (What is a race condition?).

BoolFlag is used here to indicate when the processing starts, so it is probably accessed (setValue and getValue or similar) by different threads. If we have a statement like:

// b is a BoolFlag
if (b.getValue()) { // (1)
    b.setValue(false); // (2)
}

Then it is possible that two threads succeed the test at (1) before (2) was executed. So to make the code thread-safe those two operations have to be executed in one step, they have to be atomic. Java provides java.util.concurrent.atomic.* classes for this, e.g. AtomicBoolean which have a function compareAndSet(expectedValue, newValue) which will only return true if the operation succeeded. Of course you could also write it yourself by using synchronized as you mentioned, adding a public synchronized boolean compareAndSet(...) to the BoolFlag implementation.

And before I tell any lies, this seems also worth reading Volatile boolean vs AtomicBoolean

Community
  • 1
  • 1
maraca
  • 8,468
  • 3
  • 23
  • 45
  • In your case synchronizing get and set seems to be enough. Just want to make you aware of the problems with concurrency. – maraca May 15 '15 at 19:30