Hi Berco,
Thanks for the feedback.
You’‘re welcome. Unfortunately I am pretty busy myself at the moment, so that I can’'t provide you with an implementation.
I don’‘t know the answers without trying, but I’'d like to give you some pointers, which may or may not help you.
I don’'t know anything about MIDP. What exactly are the constraints? You can read and write, but not at the same time?
So it is kind of half-duplex?
I’'m trying to figure out what exactly to synchronize in my code.
In PacketWriter.parsePackets() I could do the following:
You mean writePackets(), right?
synchronized(connection) {
while (!done) {
Packet packet = nextPacket();
writer.write(packet.toXML());
writer.flush();
}
}
a) Synchronisation, should be as granular as possible, so I would try something like this:
existing code:
while (!done) {
Packet packet = nextPacket();
if (packet != null) {
synchronized (writer) {
writer.write(packet.toXML());
writer.flush();
}
}
}
You just want to make sure that the actual write over the wire won’'t happen at the same time as a read,
so I would change in “writer” to “connection”.
You also need to do that in KeepAliveTask.run().
Be aware that there are some more calls to write.xx without any synchronisation in the existing code. This seems to be opening and closing the stream. Not sure if a problem can arise here. Needs some brain cycles.
Are you using eclipse? Hit CTRLSHIFTU when your cursor is on “writer”, you’'ll see all occurences at once.
Every write()/flush() is a candidate for synchronisation.
And in PacketReader.parsePackets() the following:
synchronized(connection) {
int eventType = parser.getEventType();
}
Ok, here is the real problem and likely the cause of the dead lock.
The read calls seem to be synchronous and blocking. This will likely result in the reader not returning any bytes anymore at some point in time and so
the lock is held infinite, because new packets will only arrive as a response when you write something to the server,
which cannot happen because it wants to have the same lock.
The only way the lock can be released is when the server sends something without the client asking for it (like an IM from another client).
So this can’'t work.
Two choices:
a) Look at the XMPPull API if you can set a timeout or use another method which takes a timeout. I doubt a bit, that this is available. In case it is
implement a spin lock like below.
b) The source of the XMLPullParser is a Reader.
PacketReader.constructor():
parser.setInput(connection.reader);
This can be the solution to your problem. It would be very clean to also go directly to the source, i.e. the code touching the wire, as in the a.m.
code.
Create a class like
public class CooperativeReader extends Reader {
private Reader source;
private Object lock;
public CooperativeReader(Reader source, Object lock) {
this. … blablba
}
public int read(char cbuf, int off, int len) throws IOException {
synchronized(lock) {
while(true) {
if (source.isReady())
return source.read(cbuf, off, len);
lock.wait(2000); // This releases the lock!
}
}
}
About isReady():
/**
- Tell whether this stream is ready to be read.
-
@return True if the next read() is guaranteed not to block for input,
-
false otherwise. Note that returning false does not guarantee that the
-
next read will block.
- @exception IOException If an I/O error occurs
*/
public boolean ready() throws IOException {
In the constructor do the following:
PacketReader.constructor():
parser.setInput(new CooperativeReader(connection.reader, connection));
You should also add “connection.notifyAll();” as the last statement in the synchronized
blocks in the PacketWriter to let this one wake up earlier then after 2000 ms. For tests it would be enought to reduce 2000 ms to 100 ms.
If this works, we need to add something to make it more production stable, like adding a timeout to read or checking for close() etc.
Will see about that then.
I might be totally wrong. I didn’'t test that, but it makes sense to me. I have been wrong before though and I am in a hurry right now.
I thought this would sync the reading/writing by using
the lock (a monitor) on the connection object, but it
appears to result in a deadlock. That might be due to
the other synchronizations going on, but that’'s pretty
difficult to find out.
Well, first of all: Running into deadlocks so easily is is a good thing ™.
Second best to the real solution
The real problems arise when you find deadlocks just in some hard to reproduce constellations.
These things are really funny after you
shipped
Cheers,
Mariano