test_addresses fails with Glib2 and Gtk2 reactors

[exarkun]

twisted.internet.test.test_tcp.TCPClientTestsBuilder_Glib2Reactor.test_addresses and twisted.internet.test.test_tcp.TCPClientTestsBuilder_Gtk2Reactor.test_addresses have been failing on the Windows builders since that host was updated.

The old versions of gtk and pygtk, with which those tests were passing, were 2.12.9 and 2.12.1 respectively (this can be seen on an old build's version output, eg < http://buildbot.twistedmatrix.com/builders/winxp32-py2.5-select/builds/450/steps/shell/logs/versions>).

The new versions, under which the tests fail, are 2.14.7 and 2.12.1.

This suggests there may have been a Gtk regression. I haven't investigated, though.

[therve]

It looks like it's failing with the select reactor as well now.

[exarkun]

Where did you see that?

[therve]

Sorry I got confused.

[therve]

The following changeset looks highly suspect, though I'm not really able to spot the error not have the build system to try to fix it:

2007-07-06  Tor Lillqvist  <tml@novell.com>
    
        * glib/giowin32.c (g_io_win32_check): When WSAEnumNetworkEvents()
        signals FD_CONNECT that means that the connection attempt
        finished, either successfully or failed. Test explicitly whether
        the connnection succeeded and set either G_IO_OUT if it did,
        G_IO_ERR|G_IO_HUP if it failed.
    
        Make sure we never set both G_IO_OUT and G_IO_HUP simultaneously
        because in Unix poll(2) POLLOUT and POLLHUP are mutually
        exclusive.
    
        Ignore whether the caller wants to watch G_IO_HUP or not. Always
        select for FD_CLOSE because Unix poll(2) also ignores whether
        POLLHUP in set the requested events bitmask or not.
    
    
    svn path=/trunk/; revision=5600

diff --git a/glib/giowin32.c b/glib/giowin32.c
index 8632209..83cb86c 100644
--- a/glib/giowin32.c
+++ b/glib/giowin32.c
@@ -822,8 +822,7 @@ g_io_win32_prepare (GSource *source,
        event_mask |= (FD_READ | FD_ACCEPT);
       if (watch->condition & G_IO_OUT)
        event_mask |= (FD_WRITE | FD_CONNECT);
-      if (watch->condition & G_IO_HUP)
-       event_mask |= FD_CLOSE;
+      event_mask |= FD_CLOSE;
 
       if (channel->event_mask != event_mask /* || channel->event != watch->pollfd.fd*/)
        {
@@ -936,13 +935,32 @@ g_io_win32_check (GSource *source)
       watch->pollfd.revents = 0;
       if (channel->last_events & (FD_READ | FD_ACCEPT))
        watch->pollfd.revents |= G_IO_IN;
-      if (channel->last_events & (FD_WRITE | FD_CONNECT))
+      if (channel->last_events & FD_WRITE)
        watch->pollfd.revents |= G_IO_OUT;
-      if (watch->pollfd.revents == 0 && (channel->last_events & (FD_CLOSE)))
-       watch->pollfd.revents |= G_IO_HUP;
+      else
+       {
+         /* We have called WSAEnumNetworkEvents() above but it didn't
+          * set FD_WRITE.
+          */
+         if (events.lNetworkEvents & FD_CONNECT)
+           {
+             if (events.iErrorCode[FD_CONNECT_BIT] == 0)
+               watch->pollfd.revents |= G_IO_OUT;
+             else
+               watch->pollfd.revents |= (G_IO_HUP | G_IO_ERR);
+           }
+         if (watch->pollfd.revents == 0 && (channel->last_events & (FD_CLOSE)))
+           watch->pollfd.revents |= G_IO_HUP;
+       }
 
-      if (!channel->write_would_have_blocked && (channel->event_mask & FD_WRITE))
-       watch->pollfd.revents |= G_IO_OUT; /* This sucks but... */
+      /* Regardless of WSAEnumNetworkEvents() result, if watching for
+       * writability, unless last write would have blocked set
+       * G_IO_OUT. But never set both G_IO_OUT and G_IO_HUP.
+       */
+      if (!(watch->pollfd.revents & G_IO_HUP) &&
+         !channel->write_would_have_blocked &&
+         (channel->event_mask & FD_WRITE))
+       watch->pollfd.revents |= G_IO_OUT;
 
       return ((watch->pollfd.revents | buffer_condition) & watch->condition);

[therve]

I have opened  https://bugzilla.gnome.org/show_bug.cgi?id=602012 upstream.

[exarkun]

This is blocking the resolution of #745.

[therve]

(In [28322]) Branching to 'gtk2-sources-3925'

[jkakar]

[1]

Please add docstrings to the following methods:

_add addReader addWriter getReaders getWriters removeAll _remove removeReader removeWriter

I see that IReactorFDSet has nice documentation for the public methods I listed above. Given this, maybe docstrings aren't necessary, but if nothing else it'd be nice to have a link to the interface so that you can get to it easily in the API documentation.

Other than that, the changes look fine to me.

[exarkun]

1. There should be something added to the Gtk2Reactor class docstring for the new _sources attribute as well.
2. In _remove there's a line, if not source in primary: which would be slightly nicer as if source not in primary: I think, though the behavior is the same either way.
3. twisted.internet.gtk2reactor.Gtk2Reactor.getWriters isn't valid epytext

[therve]

(In [28356]) Merge gtk2-sources-3925

Author: therve Reviewers: jkakar, exarkun Fixes #3925

Change the Gtk2Reactor to keep only one watch per FD source instead of one for reading and one for writing. It fixes one misbehavior under Windows.