Bug 31621

Summary:	file transfer not working on Windows
Product:	Telepathy	Reporter:	Thomas Flüeli <tflueeli>
Component:	gabble	Assignee:	Telepathy bugs list <telepathy-bugs>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	tflueeli
Version:	0.10	Keywords:	patch
Hardware:	Other
OS:	Windows (All)
See Also:	https://bugzilla.gnome.org/show_bug.cgi?id=338943
Whiteboard:
i915 platform:		i915 features:
Attachments:	gabble debug output (sender) gabble debug output (receiver) gabble debug output (sender) with the mentioned changes gabble debug output (receiver) with the mentioned changes add workaround for GLib bug on Windows fix TCP connection establishment on Windows

Description Thomas Flüeli 2010-11-14 08:30:07 UTC

From the logs I can see that establishing a socks5 transport fails and falling back to IBB doesn't work either. So there are probably two problems.

I've looked a bit into the socks5 initiation and found that connect() in try_to_connect() in gibber-tcp-transport.c returns -1 and WSAGetLastError() returns WSAEWOULDBLOCK. Sjoerd suggested on IRC to add a check to gibber_connect_errno_requires_retry(). With that change I get another error code, namely WSAEISCONN. If a add this to the check if connect() was successful it works a bit better. A socks5 auth request is sent but after that it stalls.

I also entcounter the socks5 problem with stream tubes, using Dario Freddi's telepathy-qt4 high-level-tubes branch.

The same code (file transfer and stream tubes) works well on Linux.

Comment 1 Thomas Flüeli 2010-11-14 08:31:44 UTC

Created attachment 40270 [details]
gabble debug output (sender)

Comment 2 Thomas Flüeli 2010-11-14 08:32:29 UTC

Created attachment 40271 [details]
gabble debug output (receiver)

Comment 3 Thomas Flüeli 2010-11-14 08:34:59 UTC

Created attachment 40272 [details]
gabble debug output (sender) with the mentioned changes

Comment 4 Thomas Flüeli 2010-11-14 08:35:38 UTC

Created attachment 40273 [details]
gabble debug output (receiver) with the mentioned changes

Comment 5 Thomas Flüeli 2010-11-14 08:44:26 UTC

I forgot to mention that I've added some more debug output, so don't be shocked when looking at the logs ;)

Comment 6 Thomas Flüeli 2011-01-09 03:55:57 UTC

I finally had some time to look closer at this issue. Wireshark tells me that gabble-r (instance that receives the file/stream-tube) successfully establishes a TCP connection to gabble-s (the sender instance). Then three bytes are sent (socks5 auth request). These three bytes are captured by wireshark but are never received by gabble-s.

From the log (gabble-s) and code I get the following:

- listener accepts new connection
- socket is set to non-blocking mode
- watches are added (one for G_IO_IN, one for G_IO_ERR)
- transport state is set to GIBBER_TRANSPORT_CONNECTED

I expect _channel_io_in() to be called by G_IO_IN watch once the three bytes are received, but this never happens. Any idea why?

Comment 7 Thomas Flüeli 2011-02-03 03:26:43 UTC

I tracked it down, there are two problems and for both I have a patch.

The first problem is really nasty, it's a bug (#338943) in GLib on Windows. Basically only one watch per IOChannel is supported. If multiple watches are used (as gibber does) they overwrite each other. With the first patch, file transfers using IBB are working.

The second problem is that connect() on Windows needs some more love than on UNIX.

Both patches are quite ugly and I don't know if you really want to merge them. But with both patches applied, file transfers using SOCKS5 bytestreams are working :)

Comment 8 Thomas Flüeli 2011-02-03 03:28:33 UTC

Created attachment 42895 [details] [review]
add workaround for GLib bug on Windows

This patch is a workaround for GLib bug #338943. Instead of using two distinct watches for G_IO_IN and G_IO_ERR it uses just one for both conditions on Windows. This should be fixed in GLib, but the bug is open for nearly five years.

Comment 9 Thomas Flüeli 2011-02-03 03:32:05 UTC

Created attachment 42896 [details] [review]
fix TCP connection establishment on Windows

This patch takes into account the behavior of connect() on Windows. Quote from MSDN:

"Until the connection attempt completes on a nonblocking socket, all subsequent calls to connect on the same socket will fail with the error code WSAEALREADY, and WSAEISCONN when the connection completes successfully. Due to ambiguities in version 1.1 of the Windows Sockets specification, error codes returned from connect while a connection is already pending may vary among implementations. As a result, it is not recommended that applications use multiple calls to connect to detect connection completion. If they do, they must be prepared to handle WSAEINVAL and WSAEWOULDBLOCK error values the same way that they handle WSAEALREADY, to assure robust operation."

The patch itself is rather ugly. This is due the fact that WSAGetLastError() cannot be called twice. But I tried to not clutter the code too much with ifdefs.

Comment 10 Will Thompson 2011-07-06 12:14:53 UTC

Well caught, and sorry for the patches sitting unreviewed for so long.

Both patches look fine to me. It's annoying that GLib doesn't do the hard work for us, but in the meantime (particularly given how long GLib bug #338943 has been open!) let's merge them for 0.13.3. Thanks!

http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=5c06d57058f169401449039b7148470ca1784699

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.