0

I have two threads, let's call them A and B. A is constantly looking for packets from the ObjectInputStream via the readPacket function (this would be while(true) in a thread etc)

While A is looking for these packets, I want B to write a packet to the ObjectOutputStream via the writePacket function.

But I enter a deadlock whenever I want to do this; I don't understand how two different functions can deadlock each other?

   import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.net.Socket;
import java.util.ArrayList;
import java.util.concurrent.locks.ReentrantLock;

public class ConnectionBay
{
    private Socket connection;
    private ObjectOutputStream output_stream;
    private ObjectInputStream input_stream;

    ConnectionBay(Socket socket) throws IOException{
        this.connection = socket;
        this.output_stream = new ObjectOutputStream(connection.getOutputStream());
        this.output_stream.flush();
        this.output_stream.reset();
        this.input_stream = new ObjectInputStream(connection.getInputStream());
    }
    public synchronized void writePacket(Packet packet) throws IOException{
        this.output_stream.writeObject(packet);
        this.output_stream.flush();
        this.output_stream.reset();
    }
    public synchronized Packet readPacket() throws IOException, ClassNotFoundException{
        return (Packet)this.input_stream.readObject();
    }
}

1 Answers1

0

Since you used the synchronized keyword in the readPacket and writePacket definitions, you have created synchronized methods. As described in this tutorial, it is not possible for two invocations of synchronized methods on the same object to interleave. In your case, the "object" is not the input or output stream, it is the ConnectionBay object. Normally you use synchronized methods when you want to prevent two threads from accessing any of the state in your object. Since both streams are part of a single ConnectionBay object and the synchronized methods are preventing current access to that one object, the deadlock between read and write is exactly what I would expect.

Based on your question - it sounds like what you really want is to have two separate synchronizations - one for the input stream and one for the output stream. If this is the case I would recommend using synchronized statements. The example below shows how readPacket might be modified.

public Packet readPacket() throws IOException, ClassNotFoundException{
    synchronized(this.input_stream) {
        return (Packet)this.input_stream.readObject();
    }
}

If you go with this approach (having the stream itself be the lock), you should make the "input_stream" variable final

private final ObjectInputStream input_stream;

This ensures that you can safely use input_stream as a lock since it is guaranteed not to change after construction.

Another approach would be to create two more variables to act as the locks and pair them with the stream.

private ObjectInputStream input_stream;
private final Object input_lock = new Object();
private ObjectOutputStream output_stream;
private final Object output_lock = new Object();

...

public Packet readPacket() throws IOException, ClassNotFoundException{
    synchronized(this.input_lock) {
        return (Packet)this.input_stream.readObject();
    }
}

This approach is more flexible and may be necessary if you are creating and destroying stream variables during the lifetime of the connection bay.

Don't forget to update your writePacket method with a similar change.

Guido Simone
  • 7,912
  • 2
  • 19
  • 21
  • Why would it be better to keep an object for each stream as a lock? Can't I just lock the streams directly? Ps. You have solved my problem and have made me extremely, extremely happy. I've been trying to debug this for 2 days. Thank you ever so much. Legend. – user15200182 Feb 28 '21 at 18:11
  • Glad I could help. The way your code is shown, locking the stream directly should be fine. However if you had a long running service that was connecting and disconnecting and you were creating and destroying streams on the fly, then you would probably need to create separate lock objects. The lock objects would never change even if the underlying stream objects did. – Guido Simone Feb 28 '21 at 18:48