DataOutputStream different approach

Question

I am using this code in a application for sending some string throw a socket.

public class OutgoingData {

public static DataOutputStream dos = null;
public static String toSend = "";
public static volatile boolean continuousSending = true;
public static String toSendTemp = "";

public static void startSending(final DataOutputStream d) {

    new Thread(new Runnable() {

        public void run() {
            try {
                dos = d;
                while (continuousSending) {

                    if (!toSend.equals(toSendTemp)) {
                        dos.writeUTF(toSend);
                        dos.flush();
                        toSendTemp = toSend;
                    }
                }
            } catch (IOException e) {
                e.printStackTrace();
            }

        }
    }).start();

}

And from another thread I am calling this method

    private void send(String str) {

    OutgoingData.toSend = str;
}

Are there any problems that could appear using this implementation? Excepting the case when send() is called synchronously from two threads.

I am not using something like this:

   private void send(final String str){

    new Thread(new Runnable() {

        @Override
        public void run() {
            synchronized (OutgoingData.dos) {
                try {
                    OutgoingData.dos.writeUTF(str);
                    OutgoingData.dos.flush();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }

        }
    }).start();


}

Because the system on which this code is runned, has a limit on the number of threads a process can create and takes a long time to get a lock on an object.

codelark · Accepted Answer · 2013-10-22 15:40:52Z

1

Your implementation is not thread safe:

if (!toSend.equals(toSendTemp)) {
    // toSend can be changed before this line happens
    // causing you to miss data
    dos.writeUTF(toSend);
    dos.flush();

    // or here
    toSendTemp = toSend;
}

You need some form of thread synchronization, regardless of whether or not it is "slow".

answered Oct 22, 2013 at 15:40

codelark

12.3k1 gold badge48 silver badges49 bronze badges

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

2 Comments

Peter Lawrey Over a year ago

+1 As the toSend field is not volatile, you might not ever see this field change.

Titus Over a year ago

You are right i haven't encountered this problem, but in theory, this is more than likely to happen

Peter Lawrey · Accepted Answer · 2013-10-22 16:02:01Z

A better choice rather than busy waiting on a field is to use a BlockingQueue<String> This will ensure you never miss a value, nor do you consume CPU when there is nothing to do.

A good way of wrapping up a Queue and a Thread (pool) is to use an ExecutorService which does both.

In your case, a Socket stream is a queue already so queuing writing to another queue is likely to be redundant and all you really need to buffer your output stream.

Because the system on which this code is runned, has a limit on the number of threads a process can create and takes a long time to get a lock on an object.

Creating a thread is more than 100x than creating a thread. Ideally you don't want to have either. Note: the Socket already has a write lock.

Collectives™ on Stack Overflow

DataOutputStream different approach

2 Answers 2

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related