Hi,
>From time to time during testing I've seen an issue with the new Java
client where it fails to read the version message from the server.
Previously, I had only seen it when connecting to rfbproxy, never when
connecting to a real server, so I had assumed that the problem was with
rfbproxy. However, I had a user on fractional T1 testing the trunk version
the other day and consistently reproduced this behavior when connecting to
a server running the TigerVNC 1.2 at our site (latency ~10mS). After
looking into this for the last couple days, it seems to me that there is a
potential race condition in the rfb::ConnParams::readVersion function. The
problem appears to be in the first while loop, which contains a
is->checkNoWait(1) in the loop condition:
while (is->checkNoWait(1) && verStrPos < 12) {
verStr[verStrPos++] = is->readU8();
}
However since this is the very first read that takes place in the
handshake, there should not be any data in the input buffer, and because
it's a checkNoWait a blocking read should cause the while loop to terminate
prematurely. I found that another way to reproduce this is to try
connecting to the native desktop sharing service on a Mac (which doesn't
work overall, but the handshake demonstrates the problem consistently).
Oddly though, I cannot reproduce this issue with the binary client. But
that could just be because the time between when the socket is connected
and the checkNoWait is long enough that the socket is ready to read and
doesn't block. I found that the handshake proceeds correctly with the java
client if I introduce a delay of >20mS right before the checkNoWait, which
seems to support this theory. Still, I'm surprised that this wouldn't have
been seen with the binary client before. A slow server+fast client or high
latency situation should trigger it just the same (unless there is an
intrinsic delay between the socket connecting and the checkNoWait greater
than most latency that still masks the problem). Or am I missing something
altogether here?
Assuming that it is a race condition, it's easy enough to fix. Something
like this should do the trick:
is->check(12);
while (verStrPos < 12) {
verStr[verStrPos++] = is->readU8();
}
Please let me know if I'm way off here, if it's a problem with the Java
client then I'd like to get to the bottom of it.
Thanks,
-brian
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Tigervnc-devel mailing list
Tigervnc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
