CODEWITHSUNDEEP
javamultithreadinglocking
shparkison
shparkison

Reputation: 973

Synchronizing data output stream

I'm having a problem where I have a class that gets instantiated upon a connection to server.

The method I'm having trouble with in the class looks like so:

public void sendData(byte[] dataToSend) throws IOException {
    sendLock.lock();

    int dataLength = dataToSend.length;
    dout.writeInt(dataLength);
    dout.write(dataToSend, 0, dataLength);
    dout.flush();

    sendLock.unlock();
}

Where sendLock is a ReentrantLock and dout = new DataOutputStream(socket.getOutputStream());. This will work fine with a limited number of threads, but if I have a large number of threads calling this method concurrently I get a deadlock and the program just stops.

Is there any reason a deadlock would be happening here? It doesn't make sense to me, as I've removed all other locks to rule them out and I'm down to this one. Is there anyway the flush could cause things to hang or something? It just seems like at some point it never releases the lock and I'm not sure why.

If I remove the lock I get socket errors because one thread may change the dataLength before another has a chance to write, etc. But the deadlock no longer occurs.

As a reference, here's what the run method of the Receive end looks like:

public void run() {

    while (socket != null) {
        try {
            int dataLength = din.readInt();
            byte[] data = new byte[dataLength];
            din.readFully(data, 0, dataLength);
            Event e = ef.getEvent(data);
            node.onEvent(e);    
        } catch (SocketException se) {
            System.out.println(se.getMessage());
            break;
        } catch (IOException ioe) {
            System.out.println(ioe.getMessage()) ;
            break;
        }
    }
}

Upvotes: 1

Views: 2716

Answers (1)

Charlie C
Charlie C

Reputation: 311

It's possible that one of your calls to the output stream throws an exception and sendLock.unlock() is never called. All the other threads will be left waiting forever.

Check your logs to see if one of the threads throws an exception. In your code I would use a try-catch-finally block instead of throwing IOException. This guarantees, even if something bad happens, the lock will be released so other threads can keep working.

public void sendData(byte[] dataToSend) throws IOException {
    try {
        sendLock.lock();

        int dataLength = dataToSend.length;
        dout.writeInt(dataLength);
        dout.write(dataToSend, 0, dataLength);
        dout.flush();
    }
    finally {
         sendLock.unlock();
    }
}

Upvotes: 3

Related Questions